You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/09/08 01:18:57 UTC
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Hello Dan Burkert, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8007
to review the following change.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
log_block_manager: switch from google::sparse_hash_map to sparsepp
sparsepp is updated for C++11 so it enables move semantics for the map
elements. Since the block map uses ref-counted values, being able to move them
is a big win. It also claims to be generally faster even aside from the
ability to support moves.
This improved startup time 7-8x on a real host with ~11M blocks:
Before:
I0907 17:23:50.748055 12507 fs_manager.cc:335] Time spent opening block manager: real 108.910s user 0.000s sys 0.001s
After:
I0907 17:20:42.277474 10021 fs_manager.cc:335] Time spent opening block manager: real 14.348s user 0.000s sys 0.001s
The LBM startup benchmark (1M blocks) improved less substantially but still noticeably:
Before:
I0907 17:16:54.899818 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.612s user 0.035s sys 0.002s
I0907 17:16:57.498205 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.598s user 0.039s sys 0.001s
I0907 17:17:00.100244 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.602s user 0.042s sys 0.000s
I0907 17:17:02.686638 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.586s user 0.042s sys 0.000s
I0907 17:17:05.284050 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.041s sys 0.001s
I0907 17:17:07.884395 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.600s user 0.039s sys 0.001s
I0907 17:17:10.490550 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.606s user 0.040s sys 0.001s
I0907 17:17:13.070114 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.580s user 0.039s sys 0.000s
I0907 17:17:15.667062 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.040s sys 0.001s
I0907 17:17:18.258447 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.591s user 0.042s sys 0.000s
After:
I0907 17:15:50.645310 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.570s user 0.034s sys 0.001s
I0907 17:15:52.195543 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.550s user 0.037s sys 0.001s
I0907 17:15:53.755209 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.560s user 0.037s sys 0.001s
I0907 17:15:55.263762 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.509s user 0.038s sys 0.001s
I0907 17:15:56.818748 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.555s user 0.037s sys 0.001s
I0907 17:15:58.379680 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.561s user 0.036s sys 0.001s
I0907 17:15:59.913751 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.534s user 0.038s sys 0.000s
I0907 17:16:01.461668 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.548s user 0.037s sys 0.001s
I0907 17:16:03.020823 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.559s user 0.037s sys 0.001s
I0907 17:16:04.549747 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.529s user 0.035s sys 0.001s
Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
---
M src/kudu/fs/log_block_manager.h
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
5 files changed, 35 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8007/1
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8007/1//COMMIT_MSG
Commit Message:
Line 14: This improved startup time 7-8x on a real host with ~11M blocks:
To be clear, this improvement is just from switching to sparsepp? Or with the other optimizations rolled in?
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
PS1, Line 328: SPARSEPP_PATCHLEVEL=0
: delete_if_wrong_patchlevel $SPARSEPP_SOURCE $SPARSEPP_PATCHLEVEL
: if [ ! -d "$SPARSEPP_SOURCE" ]; then
: fetch_and_expand sparsepp-${SPARSEPP_VERSION}.tar.gz
: pushd $SPARSEPP_SOURCE
: touch patchlevel-$SPARSEPP_PATCHLEVEL
: popd
: fi
If there's not one patch, you can omit the patchlevel-0 boilerplate, as well as the call to delete_if_wrong_patchlevel.
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/vars.sh
File thirdparty/vars.sh:
Line 193: # (from https://github.com/toddlipcon/sparsepp)
Why are we pulling from your fork of sparsepp and not the main repo? Have you patched it?
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/8007/1//COMMIT_MSG
Commit Message:
> Forgot to ask: we originally switched to sparse_hash_map because of it's ex
it's slightly worse (maybe 10%) than sparse_hash_map but still way better than std::unordered_map. Generally the block map isn't the worst consumer of memory.
The original commit said that 1M blocks took 9M with sparse_hash_map vs 24M with unordered_map. I'd expect this one to use maybe 11M based on the graphs at https://github.com/greg7mdp/sparsepp/blob/master/bench.md
The bigger win of these maps vs unordered_map is that it doesn't allocate any big single array for buckets. Instead it does a lot of smaller allocations. This helps avoid fragmentation issues, etc.
I'll add some color to the commit message.
Line 14: This improved startup time 7-8x on a real host with ~11M blocks:
> To be clear, this improvement is just from switching to sparsepp? Or with t
just this. the copy-vs-move is a big win because the copies involved refcounts (i.e atomic operations) and those probably inhibited a lot of memory load speculation, etc, with this random-access memory workload.
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
PS1, Line 328: SPARSEPP_PATCHLEVEL=0
: delete_if_wrong_patchlevel $SPARSEPP_SOURCE $SPARSEPP_PATCHLEVEL
: if [ ! -d "$SPARSEPP_SOURCE" ]; then
: fetch_and_expand sparsepp-${SPARSEPP_VERSION}.tar.gz
: pushd $SPARSEPP_SOURCE
: touch patchlevel-$SPARSEPP_PATCHLEVEL
: popd
: fi
> If there's not one patch, you can omit the patchlevel-0 boilerplate, as wel
I thought we wanted to start putting these in so that, if in the future we add a patch, and then rebase back to this patchlevel 0, it will be smart enough to rm and rebuild.
http://gerrit.cloudera.org:8080/#/c/8007/1/thirdparty/vars.sh
File thirdparty/vars.sh:
Line 193: # (from https://github.com/toddlipcon/sparsepp)
> Why are we pulling from your fork of sparsepp and not the main repo? Have y
yea but since then they pulled my commit in, so I'll revert back to the upstream.
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8007
to look at the new patch set (#2).
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
log_block_manager: switch from google::sparse_hash_map to sparsepp
sparsepp is updated for C++11 so it enables move semantics for the map
elements. Since the block map uses ref-counted values, being able to move them
is a big win. It also claims to be generally faster even aside from the
ability to support moves.
According to [1] this data structure uses ~10% more memory than
google::sparse_hash_map. However, previous measurement indicated that 1M
blocks used about 9MB of memory, so this isn't a major consumer as far
as the overall system is concerned. It seems worth a few extra MB of
memory in order to make substantial startup time improvements.
Despite slightly more memory usage, it's still significantly better than
std::unordered_map, and also shares the benefit of avoiding any large
allocations. (std::unordered_map needs a contiguous allocation for the
buckets array).
This patch alone improved startup time 7-8x on a real host with ~11M blocks:
Before:
I0907 17:23:50.748055 12507 fs_manager.cc:335] Time spent opening block manager: real 108.910s user 0.000s sys 0.001s
After:
I0907 17:20:42.277474 10021 fs_manager.cc:335] Time spent opening block manager: real 14.348s user 0.000s sys 0.001s
The LBM startup benchmark (1M blocks) improved less substantially but still noticeably:
Before:
I0907 17:16:54.899818 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.612s user 0.035s sys 0.002s
I0907 17:16:57.498205 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.598s user 0.039s sys 0.001s
I0907 17:17:00.100244 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.602s user 0.042s sys 0.000s
I0907 17:17:02.686638 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.586s user 0.042s sys 0.000s
I0907 17:17:05.284050 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.041s sys 0.001s
I0907 17:17:07.884395 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.600s user 0.039s sys 0.001s
I0907 17:17:10.490550 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.606s user 0.040s sys 0.001s
I0907 17:17:13.070114 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.580s user 0.039s sys 0.000s
I0907 17:17:15.667062 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.040s sys 0.001s
I0907 17:17:18.258447 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.591s user 0.042s sys 0.000s
After:
I0907 17:15:50.645310 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.570s user 0.034s sys 0.001s
I0907 17:15:52.195543 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.550s user 0.037s sys 0.001s
I0907 17:15:53.755209 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.560s user 0.037s sys 0.001s
I0907 17:15:55.263762 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.509s user 0.038s sys 0.001s
I0907 17:15:56.818748 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.555s user 0.037s sys 0.001s
I0907 17:15:58.379680 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.561s user 0.036s sys 0.001s
I0907 17:15:59.913751 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.534s user 0.038s sys 0.000s
I0907 17:16:01.461668 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.548s user 0.037s sys 0.001s
I0907 17:16:03.020823 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.559s user 0.037s sys 0.001s
I0907 17:16:04.549747 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.529s user 0.035s sys 0.001s
[1] https://github.com/greg7mdp/sparsepp/blob/master/bench.md
Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
---
M src/kudu/fs/log_block_manager.h
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
5 files changed, 35 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/8007/2
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
log_block_manager: switch from google::sparse_hash_map to sparsepp
sparsepp is updated for C++11 so it enables move semantics for the map
elements. Since the block map uses ref-counted values, being able to move them
is a big win. It also claims to be generally faster even aside from the
ability to support moves.
According to [1] this data structure uses ~10% more memory than
google::sparse_hash_map. However, previous measurement indicated that 1M
blocks used about 9MB of memory, so this isn't a major consumer as far
as the overall system is concerned. It seems worth a few extra MB of
memory in order to make substantial startup time improvements.
Despite slightly more memory usage, it's still significantly better than
std::unordered_map, and also shares the benefit of avoiding any large
allocations. (std::unordered_map needs a contiguous allocation for the
buckets array).
This patch alone improved startup time 7-8x on a real host with ~11M blocks:
Before:
I0907 17:23:50.748055 12507 fs_manager.cc:335] Time spent opening block manager: real 108.910s user 0.000s sys 0.001s
After:
I0907 17:20:42.277474 10021 fs_manager.cc:335] Time spent opening block manager: real 14.348s user 0.000s sys 0.001s
The LBM startup benchmark (1M blocks) improved less substantially but still noticeably:
Before:
I0907 17:16:54.899818 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.612s user 0.035s sys 0.002s
I0907 17:16:57.498205 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.598s user 0.039s sys 0.001s
I0907 17:17:00.100244 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.602s user 0.042s sys 0.000s
I0907 17:17:02.686638 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.586s user 0.042s sys 0.000s
I0907 17:17:05.284050 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.041s sys 0.001s
I0907 17:17:07.884395 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.600s user 0.039s sys 0.001s
I0907 17:17:10.490550 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.606s user 0.040s sys 0.001s
I0907 17:17:13.070114 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.580s user 0.039s sys 0.000s
I0907 17:17:15.667062 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.597s user 0.040s sys 0.001s
I0907 17:17:18.258447 20839 log_block_manager-test.cc:799] Time spent reopening block manager: real 2.591s user 0.042s sys 0.000s
After:
I0907 17:15:50.645310 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.570s user 0.034s sys 0.001s
I0907 17:15:52.195543 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.550s user 0.037s sys 0.001s
I0907 17:15:53.755209 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.560s user 0.037s sys 0.001s
I0907 17:15:55.263762 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.509s user 0.038s sys 0.001s
I0907 17:15:56.818748 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.555s user 0.037s sys 0.001s
I0907 17:15:58.379680 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.561s user 0.036s sys 0.001s
I0907 17:15:59.913751 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.534s user 0.038s sys 0.000s
I0907 17:16:01.461668 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.548s user 0.037s sys 0.001s
I0907 17:16:03.020823 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.559s user 0.037s sys 0.001s
I0907 17:16:04.549747 20302 log_block_manager-test.cc:799] Time spent reopening block manager: real 1.529s user 0.035s sys 0.001s
[1] https://github.com/greg7mdp/sparsepp/blob/master/bench.md
Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Reviewed-on: http://gerrit.cloudera.org:8080/8007
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/fs/log_block_manager.h
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
5 files changed, 35 insertions(+), 2 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] log block manager: switch from google::sparse hash map to sparsepp
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: switch from google::sparse_hash_map to sparsepp
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8007/1//COMMIT_MSG
Commit Message:
Forgot to ask: we originally switched to sparse_hash_map because of it's excellent memory footprint. Is sparsepp better, worse, or the same?
--
To view, visit http://gerrit.cloudera.org:8080/8007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7397f9cd418782caecf8b2dae2c7bfe2c0e6215c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes