[kudu-CR] [consensus] simplify RpcPeerProxy a bit
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15049 ) Change subject: [consensus] simplify RpcPeerProxy a bit .. [consensus] simplify RpcPeerProxy a bit It's a small clean-up on the RpcPeerProxy class to avoid an extra call to operator new while preparing its HostPort parameter at call sites. Change-Id: Ie3185142ee16130f66609e88bcd3439956961c2e Reviewed-on: http://gerrit.cloudera.org:8080/15049 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_peers.h 2 files changed, 8 insertions(+), 9 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie3185142ee16130f66609e88bcd3439956961c2e Gerrit-Change-Number: 15049 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] encoding-test: clean up bitshuffle tests a little
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15043 to look at the new patch set (#2). Change subject: encoding-test: clean up bitshuffle tests a little .. encoding-test: clean up bitshuffle tests a little Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 --- M src/kudu/cfile/encoding-test.cc 1 file changed, 39 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15043/2 -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15042 to look at the new patch set (#2). Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 103 insertions(+), 98 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/15042/2 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15044 to look at the new patch set (#2). Change subject: cfile: clean up encoding-test to use fewer templates .. cfile: clean up encoding-test to use fewer templates This test made heavy use of templates, which made things overly complicated and hard to follow. All of the block builders/decoders already implement common interfaces, so we can use runtime polymorphism instead for the majority of code here. This also cleans up the block builder/decoder factories to use unique_ptr out-parameters instead of raw pointers. Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/type_encodings.cc M src/kudu/cfile/type_encodings.h 8 files changed, 274 insertions(+), 321 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/15044/2 -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [build] Make IWYU scipts compatible with Python 3
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15031 ) Change subject: [build] Make IWYU scipts compatible with Python 3 .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15031/2/build-support/iwyu.py File build-support/iwyu.py: http://gerrit.cloudera.org:8080/#/c/15031/2/build-support/iwyu.py@178 PS2, Line 178: stream = BytesIO(iwyu_output) this isn't working for me on python 2.7 -- it seems like 'output' here is a unicode object (since you used decode on line 130) so this needs to be StringIO -- To view, visit http://gerrit.cloudera.org:8080/15031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I979e96f66261146571ffc45f8222ad0fc15d1073 Gerrit-Change-Number: 15031 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 Jan 2020 06:39:30 + Gerrit-HasComments: Yes
[kudu-CR] encoding-test: clean up bitshuffle tests a little
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/15043 )
Change subject: encoding-test: clean up bitshuffle tests a little
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@719
PS1, Line 719: 1
> Do we explicitly want it to have the same seed every run?
Done
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@721
PS1, Line 721: for (int i = 0; i < count; i++) {
> nit: maybe, use the same 'for (auto& e : ints) ...' construct here as well?
Done
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@742
PS1, Line 742: int32_t
> int64_t ?
woops, good catch
--
To view, visit http://gerrit.cloudera.org:8080/15043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284
Gerrit-Change-Number: 15043
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 16 Jan 2020 06:24:07 +
Gerrit-HasComments: Yes
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/15042 )
Change subject: cfile: change BlockBuilder API to yield a vector of Slices
..
Patch Set 1:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc
File src/kudu/cfile/binary_dict_block.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc@65
PS1, Line 65: void BinaryDictBlockBuilder::Reset() {
> Is it necessary to clear the buffer_ member as well?
nope, because it's always completely overwritten in Finish(). I'll rename it to
header_buf
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc@85
PS1, Line 85: data_slices.insert(data_slices.begin(), Slice(buffer_));
: *slices = std::move(data_slices);
> Inserting into the beginning of a vector is not the best option from the pe
isn't that equivalent from a performnace perspective? in either case, you're
ending up copying all of the data_slices once (in your case to append to ret,
in my case when inserting at the beginning).
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.h
File src/kudu/cfile/binary_plain_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.h@85
PS1, Line 85: static const size_t kHeaderSize = sizeof(uint32_t) * 3;
> nit: would it benefit from adding constexpr ?
Done
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.cc
File src/kudu/cfile/binary_plain_block.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.cc@59
PS1, Line 59: buffer_.resize(kHeaderSize);
: buffer_.reserve(options_->storage_attributes.cfile_block_size);
> Would it be more optimal to switch these two lines?
Done
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/bshuf_block.h
File src/kudu/cfile/bshuf_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/bshuf_block.h@53
PS1, Line 53: using std::vector;
> Is it really needed?
yea not sure how it got there, thanks
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc
File src/kudu/cfile/cfile_writer.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc@399
PS1, Line 399: null_bitmap
> nit: wrap into std::move() ?
I think given Slice is a simple value type there isn't any benefit to moving vs
copying (otherwise I think clang-tidy would complain here)
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc@401
PS1, Line 401: for (const auto& s : data_slices) {
: v.emplace_back(s);
: }
> nit: consider instead
Done
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc@127
PS1, Line 127: contiguous_buf_
> Is it possible to use just a local variable instead this member field and m
nope, because the Slice doesn't own the pointed-to memory, so it has to outlast
the function call scope
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/rle_block.h
File src/kudu/cfile/rle_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/rle_block.h@78
PS1, Line 78: buf_
> Slice(buf_) ? Or the compiler is smart enough to wrap it into a Slice itse
yea, Slice has an implicit constructor for faststring apparently
--
To view, visit http://gerrit.cloudera.org:8080/15042
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db
Gerrit-Change-Number: 15042
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 16 Jan 2020 06:12:59 +
Gerrit-HasComments: Yes
[kudu-CR] [tserver] replace gscoped ptr with unique ptr in Scanner
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15048 ) Change subject: [tserver] replace gscoped_ptr with unique_ptr in Scanner .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG@10 PS1, Line 10: and avoid extra call to operator new > It's at https://gerrit.cloudera.org/#/c/15048/1/src/kudu/tserver/tablet_ser Oops, I reviewed the first two files but not the third. My bad. -- To view, visit http://gerrit.cloudera.org:8080/15048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieac533565f96773cc450de521703c21534020596 Gerrit-Change-Number: 15048 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 06:00:06 + Gerrit-HasComments: Yes
[kudu-CR] tools: avoid extra 100ms sleep at end of table scan
Hello Yingchun Lai, Kudu Jenkins, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15040 to look at the new patch set (#2). Change subject: tools: avoid extra 100ms sleep at end of table scan .. tools: avoid extra 100ms sleep at end of table scan Prior to this patch, the table scan tool printed a status output every 5 seconds by starting a separate monitor thread. This thread's main loop would only check for the scan completion every 100ms, so after the scans completed, the process could hang for up to an extra 100ms before exiting. This changes the printing to happen from the main thread instead, with the waiting based on the actual scan completion. Now the tool exits immediately when the scan completes. Example output: Before: todd@turbo:~/kudu$ time ./build/latest/bin/kudu table scan localhost sequences | wc -l 79 real 0m0.180s user 0m0.041s sys 0m0.021s After: todd@turbo:~/kudu$ time ./build/latest/bin/kudu table scan localhost sequences | wc -l 79 real 0m0.078s user 0m0.055s sys 0m0.005s Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 --- M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 2 files changed, 6 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/15040/2 -- To view, visit http://gerrit.cloudera.org:8080/15040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 Gerrit-Change-Number: 15040 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2971 p2: add basic protobuf msg
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14426 ) Change subject: KUDU-2971 p2: add basic protobuf msg .. Patch Set 10: Verified+1 Unrelated flaky test. -- To view, visit http://gerrit.cloudera.org:8080/14426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If02ab0f9bc1bc73f9c1a46d96bdef6725b8f6954 Gerrit-Change-Number: 14426 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 05:58:35 + Gerrit-HasComments: No
[kudu-CR] KUDU-2971 p2: add basic protobuf msg
Hao Hao has removed a vote on this change. Change subject: KUDU-2971 p2: add basic protobuf msg .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/14426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: If02ab0f9bc1bc73f9c1a46d96bdef6725b8f6954 Gerrit-Change-Number: 14426 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15044 ) Change subject: cfile: clean up encoding-test to use fewer templates .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15044/1/src/kudu/cfile/type_encodings.h File src/kudu/cfile/type_encodings.h: http://gerrit.cloudera.org:8080/#/c/15044/1/src/kudu/cfile/type_encodings.h@46 PS1, Line 46: * nit: while you are here, maybe stick * and & to the type in the changes lines? -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 16 Jan 2020 05:52:49 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] replace gscoped ptr with unique ptr in Scanner
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15048 ) Change subject: [tserver] replace gscoped_ptr with unique_ptr in Scanner .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG@10 PS1, Line 10: and avoid extra call to operator new > Not seeing where this is. It's at https://gerrit.cloudera.org/#/c/15048/1/src/kudu/tserver/tablet_service.cc@a2165 -- To view, visit http://gerrit.cloudera.org:8080/15048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieac533565f96773cc450de521703c21534020596 Gerrit-Change-Number: 15048 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 05:46:34 + Gerrit-HasComments: Yes
[kudu-CR] tools: avoid extra 100ms sleep at end of table scan
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/15040 ) Change subject: tools: avoid extra 100ms sleep at end of table scan .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 Gerrit-Change-Number: 15040 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 16 Jan 2020 05:43:15 + Gerrit-HasComments: No
[kudu-CR] tools: avoid extra 100ms sleep at end of table scan
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15040 ) Change subject: tools: avoid extra 100ms sleep at end of table scan .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15040/1/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/15040/1/src/kudu/tools/table_scanner.cc@614 PS1, Line 614: thread_pool_->Wait(); > This Wait() won't be needed now. Done -- To view, visit http://gerrit.cloudera.org:8080/15040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 Gerrit-Change-Number: 15040 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 16 Jan 2020 05:38:30 + Gerrit-HasComments: Yes
[kudu-CR] [python] Fix Python 3 syntax issues
Adar Dembo has removed a vote on this change. Change subject: [python] Fix Python 3 syntax issues .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [python] Fix Python 3 syntax issues
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. [python] Fix Python 3 syntax issues This patch fixes many of the Python 3 incompatibilities in the various Kudu Python scripts while still maintiaining Python 2 compatibility. Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Reviewed-on: http://gerrit.cloudera.org:8080/15037 Reviewed-by: Bankim Bhavsar Reviewed-by: Adar Dembo Tested-by: Adar Dembo --- M build-support/build_source_release.py M build-support/check_compatibility.py M build-support/clang_tidy_gerrit.py M build-support/dist_test.py M build-support/iwyu.py M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py M build-support/parse_test_failure.py M build-support/push_to_asf.py M build-support/release/check-rat-report.py M build-support/run_dist_test.py M build-support/test_result_server.py M examples/python/graphite-kudu/kudu/kudu_graphite.py M src/kudu/benchmarks/wal_hiccup-parser.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/graph-metrics.py M src/kudu/scripts/max_skew_estimate.py M src/kudu/scripts/parse_metrics_log.py M src/kudu/scripts/write-jobs-stats-to-mysql.py 19 files changed, 63 insertions(+), 48 deletions(-) Approvals: Bankim Bhavsar: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [python] Fix Python 3 syntax issues
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. Patch Set 2: Verified+1 Overriding Jenkins, unrelated test failure. -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 04:39:28 + Gerrit-HasComments: No
[kudu-CR] [build] Fix the RAT report
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15038 ) Change subject: [build] Fix the RAT report .. [build] Fix the RAT report The patch fixes the RAT report by excluding license checks for the testdata files. Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b Reviewed-on: http://gerrit.cloudera.org:8080/15038 Reviewed-by: Adar Dembo Reviewed-by: Attila Bukor Tested-by: Kudu Jenkins --- M build-support/release/rat_exclude_files.txt 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Attila Bukor: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b Gerrit-Change-Number: 15038 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/15034/4/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: http://gerrit.cloudera.org:8080/#/c/15034/4/src/kudu/common/column_predicate.h@152 PS4, Line 152: std::vector&& bfs, Here and below you don't need the rvalue reference; could just pass std::vector<...> as-is and still move it. http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h File src/kudu/util/block_bloom_filter.h: http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@78 PS3, Line 78: // Initialize the internal data structures using the supplied arguments. : // Useful for de-serializing the BlockBloomFilter. : Status Init(int log_space_bytes, const void* src_data, size_t src_len, bool always_false); > Passing the entire kudu::ColumnPredicatePB_BlockBloomFilter would require i Good point. What do you think about moving the PB representation of BlockBloomFilter into kudu_util? There's some precedence for that (see hash.proto). http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc File src/kudu/util/block_bloom_filter.cc: http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc@108 PS3, Line 108: > Done Ths Init variant calls InitInternal (memset on L91) and then calls memcpy (L115). Likewise, the other Init variant calls InitInternal (memset) and does another memset on L98. So it doesn't seem like we've gained here; if anything, we're duplicating work more now. Am I missing something? -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 16 Jan 2020 04:35:30 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] replace gscoped ptr with unique ptr in Scanner
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15048 ) Change subject: [tserver] replace gscoped_ptr with unique_ptr in Scanner .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15048/1//COMMIT_MSG@10 PS1, Line 10: and avoid extra call to operator new Not seeing where this is. -- To view, visit http://gerrit.cloudera.org:8080/15048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieac533565f96773cc450de521703c21534020596 Gerrit-Change-Number: 15048 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 04:28:35 + Gerrit-HasComments: Yes
[kudu-CR] [consensus] simplify RpcPeerProxy a bit
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15049 ) Change subject: [consensus] simplify RpcPeerProxy a bit .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3185142ee16130f66609e88bcd3439956961c2e Gerrit-Change-Number: 15049 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jan 2020 04:27:07 + Gerrit-HasComments: No
[kudu-CR] [consensus] simplify RpcPeerProxy a bit
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15049 Change subject: [consensus] simplify RpcPeerProxy a bit .. [consensus] simplify RpcPeerProxy a bit It's a small clean-up on the RpcPeerProxy class to avoid an extra call to operator new while preparing its HostPort parameter at call sites. Change-Id: Ie3185142ee16130f66609e88bcd3439956961c2e --- M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_peers.h 2 files changed, 8 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/15049/1 -- To view, visit http://gerrit.cloudera.org:8080/15049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie3185142ee16130f66609e88bcd3439956961c2e Gerrit-Change-Number: 15049 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [tserver] replace gscoped ptr with unique ptr in Scanner
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15048 Change subject: [tserver] replace gscoped_ptr with unique_ptr in Scanner .. [tserver] replace gscoped_ptr with unique_ptr in Scanner This is a small clean up to replace gscoped_ptr with std::unique_ptr and avoid extra call to operator new. I also moved the Scanner::IsInitialized() method into the private section. Change-Id: Ieac533565f96773cc450de521703c21534020596 --- M src/kudu/tserver/scanners.cc M src/kudu/tserver/scanners.h M src/kudu/tserver/tablet_service.cc 3 files changed, 39 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/15048/1 -- To view, visit http://gerrit.cloudera.org:8080/15048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ieac533565f96773cc450de521703c21534020596 Gerrit-Change-Number: 15048 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. KUDU-3011 p6: don't transfer leadership to quiescing followers When a tablet server is quiescing, any followers hosted on it should not be considered candidates to be the leader's successor. A quiescing follower already would never become a leader because it would reject the StartElection request immediately. This patch improves upon this by nipping such requests in the bud. Followers will now send along with their ConsensusResponses whether or not they're quiescing, and if they are, the leader will know not to transfer leadership to it. I considered another approach to reducing the number of fruitless RPCs sent -- simply throttling the interval with which a leader can send StartElection requests. I opted to go with the current approach since it is more complete with regards to preventing extraneous StartElection requests. Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Reviewed-on: http://gerrit.cloudera.org:8080/15035 Reviewed-by: Adar Dembo Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 56 insertions(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] encoding-test: clean up bitshuffle tests a little
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/15043 )
Change subject: encoding-test: clean up bitshuffle tests a little
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@719
PS1, Line 719: 1
Do we explicitly want it to have the same seed every run?
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@721
PS1, Line 721: for (int i = 0; i < count; i++) {
nit: maybe, use the same 'for (auto& e : ints) ...' construct here as well?
http://gerrit.cloudera.org:8080/#/c/15043/1/src/kudu/cfile/encoding-test.cc@742
PS1, Line 742: int32_t
int64_t ?
Or this is not a typo and it's necessary to have int32 limits for this test?
--
To view, visit http://gerrit.cloudera.org:8080/15043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284
Gerrit-Change-Number: 15043
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 16 Jan 2020 02:46:37 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. KUDU-3011 p5: transfer leadership when quiescing This amends the behavior of quiescing such that when a tablet server is quiescing, it will transfer leadership to a caught-up follower as soon as it can. While in this state, unlike while in a graceful stepdown period, the tablet can still be written to, as to not obstruct on-going workloads. Tests are added to exercise: - The basic behavior: even without injecting any errors that might cause elections, a quiescing leader will relinquish leadership. - The behavior when there are followers being caught up. In such cases, the leader won't immediately relinquish leadership -- instead, it will wait for the followers to catch up before stepping down. - The behavior when being written to. The fact that a leader is quiescing shouldn't affect its ability to be written to. - The behavior of the PeerMessageQueue when responding to various peer responses. I also removed some election-causing injection in a couple existing tests that was previously required to transfer leadership while quiescing. Note: right now, if all tablet servers are quiescing while there is a write workload on-going, a large number of StartElection requests will be sent from the leaders to the followers. A follow-up patch will address this. Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Reviewed-on: http://gerrit.cloudera.org:8080/15012 Reviewed-by: Adar Dembo Reviewed-by: Alexey Serbin Tested-by: Andrew Wong --- M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 308 insertions(+), 27 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Looks good to me, approved Andrew Wong: Verified -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 7: Verified+1 Test failure is a known flake. -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 16 Jan 2020 02:04:27 + Gerrit-HasComments: No
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Andrew Wong has removed a vote on this change. Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/15042 )
Change subject: cfile: change BlockBuilder API to yield a vector of Slices
..
Patch Set 1:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc
File src/kudu/cfile/binary_dict_block.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc@65
PS1, Line 65: void BinaryDictBlockBuilder::Reset() {
Is it necessary to clear the buffer_ member as well?
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc@85
PS1, Line 85: data_slices.insert(data_slices.begin(), Slice(buffer_));
: *slices = std::move(data_slices);
Inserting into the beginning of a vector is not the best option from the
performance perspective. Maybe, replace with:
vector ret{ Slice(buffer_) };
std::move(data_slices.begin(), data_slices.end(), std::back_inserter(ret));
*slices = std::move(ret);
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.h
File src/kudu/cfile/binary_plain_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.h@85
PS1, Line 85: static const size_t kHeaderSize = sizeof(uint32_t) * 3;
nit: would it benefit from adding constexpr ?
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.cc
File src/kudu/cfile/binary_plain_block.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_plain_block.cc@59
PS1, Line 59: buffer_.resize(kHeaderSize);
: buffer_.reserve(options_->storage_attributes.cfile_block_size);
Would it be more optimal to switch these two lines?
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/bshuf_block.h
File src/kudu/cfile/bshuf_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/bshuf_block.h@53
PS1, Line 53: using std::vector;
Is it really needed?
In general, adding 'using' into headers is not a good idea.
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc
File src/kudu/cfile/cfile_writer.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc@399
PS1, Line 399: null_bitmap
nit: wrap into std::move() ?
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/cfile_writer.cc@401
PS1, Line 401: for (const auto& s : data_slices) {
: v.emplace_back(s);
: }
nit: consider instead
std::move(data_slices.begin(), data_slices.end(), std::back_inserter(v));
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc@127
PS1, Line 127: contiguous_buf_
Is it possible to use just a local variable instead this member field and make
this method static?
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/rle_block.h
File src/kudu/cfile/rle_block.h:
http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/rle_block.h@78
PS1, Line 78: buf_
Slice(buf_) ? Or the compiler is smart enough to wrap it into a Slice itself?
--
To view, visit http://gerrit.cloudera.org:8080/15042
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db
Gerrit-Change-Number: 15042
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 16 Jan 2020 01:50:57 +
Gerrit-HasComments: Yes
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15042 to review the following change. Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 99 insertions(+), 96 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/15042/1 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin
[kudu-CR] cfile: clean up encoding-test to use fewer templates
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15044 to review the following change. Change subject: cfile: clean up encoding-test to use fewer templates .. cfile: clean up encoding-test to use fewer templates This test made heavy use of templates, which made things overly complicated and hard to follow. All of the block builders/decoders already implement common interfaces, so we can use runtime polymorphism instead for the majority of code here. This also cleans up the block builder/decoder factories to use unique_ptr out-parameters instead of raw pointers. Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/type_encodings.cc M src/kudu/cfile/type_encodings.h 7 files changed, 239 insertions(+), 289 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/15044/1 -- To view, visit http://gerrit.cloudera.org:8080/15044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iba4464c2ea41107df96c68ea61576a0ea269277a Gerrit-Change-Number: 15044 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin
[kudu-CR] encoding-test: clean up bitshuffle tests a little
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15043 to review the following change. Change subject: encoding-test: clean up bitshuffle tests a little .. encoding-test: clean up bitshuffle tests a little Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 --- M src/kudu/cfile/encoding-test.cc 1 file changed, 39 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15043/1 -- To view, visit http://gerrit.cloudera.org:8080/15043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8b7bee1f952c46c09df5102ac1916141c6935284 Gerrit-Change-Number: 15043 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15035/4/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/15035/4/src/kudu/consensus/consensus_queue-test.cc@353 PS4, Line 353: // If the peer stops reporting its server as quiescing, elections will start Thank you for adding this sub-scenario. -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 16 Jan 2020 00:53:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 16 Jan 2020 00:33:30 + Gerrit-HasComments: No
[kudu-CR] tools: avoid extra 100ms sleep at end of table scan
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15040 ) Change subject: tools: avoid extra 100ms sleep at end of table scan .. Patch Set 1: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/15040/1/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/15040/1/src/kudu/tools/table_scanner.cc@505 PS1, Line 505: *out_ << row.ToString() << "\n"; : } : out_->flush(); Nice! Didn't know about the difference between endl and \n until I looked it up :) http://gerrit.cloudera.org:8080/#/c/15040/1/src/kudu/tools/table_scanner.cc@614 PS1, Line 614: thread_pool_->Wait(); This Wait() won't be needed now. -- To view, visit http://gerrit.cloudera.org:8080/15040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 Gerrit-Change-Number: 15040 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Thu, 16 Jan 2020 00:21:37 + Gerrit-HasComments: Yes
[kudu-CR] Integrate BlockBloomFilter with ColumnPredicate on server side
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/15034 )
Change subject: Integrate BlockBloomFilter with ColumnPredicate on server side
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/15034/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15034/3//COMMIT_MSG@14
PS3, Line 14: didn't ship a client
> Nit: didn't provide public client APIs
Done
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/common/column_predicate.cc@78
PS3, Line 78: bloom_filters_.swap(*bfs);
> Could maybe pass bfs by value and std::move() into bloom_filters_ as part o
Done
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/common/common.proto
File src/kudu/common/common.proto:
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/common/common.proto@374
PS3, Line 374: optional HashAlgorithm hash_algorithm = 3 [default =
FAST_HASH];
: optional uint32 hash_seed = 4;
: optional bool always_false = 5;
> Could you doc these fields too?
Done
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h
File src/kudu/util/block_bloom_filter.h:
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@78
PS3, Line 78: // Initialize the internal data structures using the supplied
arguments.
: // Useful for de-serializing the BlockBloomFilter.
: Status Init(int log_space_bytes, const void* src_data, size_t
src_len, bool always_false);
> It'd be more clear if you called this FromPB and passed the entire BlockBlo
Passing the entire kudu::ColumnPredicatePB_BlockBloomFilter would require
including kudu/common/common.pb.h from the generated code in this kudu util
directory. That'll make importing kudu util to other projects like Impala
difficult/not possible.
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@118
PS3, Line 118: int GetLogSpaceBytes() const {
> Doc?
Done
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@121
PS3, Line 121:
: // Assign the internal directory data structure to the out
parameter "bloom_data".
: // Useful for serializing the BlockBloomFilter.
: void AssignDirectory(std::string* bloom_data) const;
> Likewise, maybe model as ToPB()?
Same as above.
For readability, moved all functions related to serialization together and
added a comment.
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc
File src/kudu/util/block_bloom_filter.cc:
http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc@108
PS3, Line 108: memcpy(directory_, src_data, src_len);
> Would be nice to avoid the memset() in the other Init() overload if possibl
Done
--
To view, visit http://gerrit.cloudera.org:8080/15034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a
Gerrit-Change-Number: 15034
Gerrit-PatchSet: 3
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: ZhangYao
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Thu, 16 Jan 2020 00:03:16 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Hello Kudu Jenkins, helifu, Yao Xu, Adar Dembo, ZhangYao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15034 to look at the new patch set (#4). Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side This change switches the implementation of the ColumnPredicate to use the BlockBloomFilter for the BloomFilter predicate on the server side. Earlier implementation was still experimental and didn't provide public client APIs that actually use this BloomFilter predicate so taken the liberty to make incompatible wire protocol changes. Most of the change involves refactoring the implementation including the unit tests. Currently only FAST_HASH algorithm is supported since 32-bit versions of MURMUR2 and CITY_HASH are not currently implemented. Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a --- M src/kudu/common/column_predicate-test.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h M src/kudu/common/common.proto M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc M src/kudu/tablet/cfile_set-test.cc M src/kudu/util/block_bloom_filter.cc M src/kudu/util/block_bloom_filter.h M src/kudu/util/hash.proto M src/kudu/util/hash_util.h 11 files changed, 441 insertions(+), 308 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/15034/4 -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu
[kudu-CR] [java] KUDU-2971: process communicates via protobuf-based protocol
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14329 )
Change subject: [java] KUDU-2971: process communicates via protobuf-based
protocol
..
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java
File
java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java:
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java@84
PS7, Line 84: // InterruptedException during the put, record the
interruption
> If the thread is interrupted, we should terminate. If the blocking queue is
We discussed this offline, and decide to exit if encounter InterruptedException
(consider it is a signal to shutdown the task, since we do not have
intentionally interrupt call in the code). We also decided to remove the
shutdown hook in Subprocessor as gracefully shutdown is not essential as we
don't maintain states in the Subprocess.
http://gerrit.cloudera.org:8080/#/c/14329/8/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java
File
java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java:
http://gerrit.cloudera.org:8080/#/c/14329/8/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java@79
PS8, Line 79: if (injectInterrupt) {
> I'm not sure I understand, the thread would be blocking on either messagePr
The problem is in the test the input stream is short, the reader task can get
EOF before being interrupted by the test thread.
http://gerrit.cloudera.org:8080/#/c/14329/7/java/kudu-subprocess/src/main/resources/log4j2.properties
File java/kudu-subprocess/src/main/resources/log4j2.properties:
PS7:
> Answering my own question: unlike our other modules (which basically provid
Ack
--
To view, visit http://gerrit.cloudera.org:8080/14329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982
Gerrit-Change-Number: 14329
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Jan 2020 23:35:40 +
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-2971: process communicates via protobuf-based protocol
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14329 to look at the new patch set (#10). Change subject: [java] KUDU-2971: process communicates via protobuf-based protocol .. [java] KUDU-2971: process communicates via protobuf-based protocol This commit adds a java tool that can communicate over a stdin/stdout pipe via protobuf-based protocol. It is useful in cases a Kudu process (e.g. master) needs to talk to third-party libraries written in Java. This tool has: 1) a single reader thread that continuously reads protobuf-based messages from the standard input and puts the messages to a FIFO blocking queue; 2) multiple writer threads that continuously retrieve the messages from the queue, process them and write the responses to the standard output. IOException is fatal and causes the program to exit, e.g. I/O errors when reading/writing to the pipe, and parsing malformed protobuf messages. If encounter InterruptedException during placing/getting messages to/from the queue, we consider it to be a signal to shutdown the task which cause the program to exit as well. To support a new protobuf message type, simply extend the 'ProtocolProcessor' interface and add the specific ProcessorMain class (similar to 'EchoProcessorMain') for the message type. Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 --- A java/kudu-subprocess/build.gradle A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/EchoProcessorMain.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/EchoProtocolProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/KuduSubprocessException.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ProtocolProcessor.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessCommandLineParser.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java A java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/Subprocessor.java A java/kudu-subprocess/src/main/resources/log4j2.properties A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/MessageTestUtil.java A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageProcessor.java A java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageReaderWriter.java M java/settings.gradle 16 files changed, 1,322 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/14329/10 -- To view, visit http://gerrit.cloudera.org:8080/14329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaf9ad24dbc9acc681284b6433836271b5b4c7982 Gerrit-Change-Number: 14329 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/15035/3/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/15035/3/src/kudu/consensus/consensus_queue.cc@1143 PS3, Line 1143: peer->remote_server_quiescing = response.has_server_quiescing() && : response.server_quiescing(); : > Do we expect transitions from the quiescent to the normal state? Is so, ma Good call. Done -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 23:33:46 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15035 to look at the new patch set (#4). Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. KUDU-3011 p6: don't transfer leadership to quiescing followers When a tablet server is quiescing, any followers hosted on it should not be considered candidates to be the leader's successor. A quiescing follower already would never become a leader because it would reject the StartElection request immediately. This patch improves upon this by nipping such requests in the bud. Followers will now send along with their ConsensusResponses whether or not they're quiescing, and if they are, the leader will know not to transfer leadership to it. I considered another approach to reducing the number of fruitless RPCs sent -- simply throttling the interval with which a leader can send StartElection requests. I opted to go with the current approach since it is more complete with regards to preventing extraneous StartElection requests. Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 --- M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 56 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/15035/4 -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] tools: avoid extra 100ms sleep at end of table scan
Hello Yingchun Lai, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15040 to review the following change. Change subject: tools: avoid extra 100ms sleep at end of table scan .. tools: avoid extra 100ms sleep at end of table scan Prior to this patch, the table scan tool printed a status output every 5 seconds by starting a separate monitor thread. This thread's main loop would only check for the scan completion every 100ms, so after the scans completed, the process could hang for up to an extra 100ms before exiting. This changes the printing to happen from the main thread instead, with the waiting based on the actual scan completion. Now the tool exits immediately when the scan completes. Example output: Before: todd@turbo:~/kudu$ time ./build/latest/bin/kudu table scan localhost sequences | wc -l 79 real 0m0.180s user 0m0.041s sys 0m0.021s After: todd@turbo:~/kudu$ time ./build/latest/bin/kudu table scan localhost sequences | wc -l 79 real 0m0.078s user 0m0.055s sys 0m0.005s Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 --- M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 2 files changed, 6 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/15040/1 -- To view, visit http://gerrit.cloudera.org:8080/15040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic7dcd8408b1dbd5322a2f5e8f4889f5f5660ece0 Gerrit-Change-Number: 15040 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/15012/7/src/kudu/integration-tests/tablet_server_quiescing-itest.cc File src/kudu/integration-tests/tablet_server_quiescing-itest.cc: http://gerrit.cloudera.org:8080/#/c/15012/7/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@525 PS7, Line 525: TestAbruptStepdownWhileAllQuiescing Thank you for adding this scenario. It's great to be explicit about the behavior to expect from the system. -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 22:49:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 7: Code-Review+2 Ah, I see you added https://gerrit.cloudera.org/#/c/15035/ Thank you for addressing that concern! -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 22:44:59 + Gerrit-HasComments: No
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/15035 )
Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers
..
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15035/3/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:
http://gerrit.cloudera.org:8080/#/c/15035/3/src/kudu/consensus/consensus_queue.cc@1143
PS3, Line 1143: if (response.has_server_quiescing()) {
: peer->remote_server_quiescing =
response.server_quiescing();
: }
Do we expect transitions from the quiescent to the normal state? Is so, maybe
either always populate the 'server_quiescing' field in the response or always
update the peer's 'remote_server_quiescing' field?
--
To view, visit http://gerrit.cloudera.org:8080/15035
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066
Gerrit-Change-Number: 15035
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 15 Jan 2020 22:37:10 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 22:22:37 + Gerrit-HasComments: No
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/15012/3/src/kudu/integration-tests/tablet_server_quiescing-itest.cc File src/kudu/integration-tests/tablet_server_quiescing-itest.cc: http://gerrit.cloudera.org:8080/#/c/15012/3/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@512 PS3, Line 512: > We chatted about this a bit, though I think this comment was left with some Yep, as soon as there is a guarantee that former leader replica doesn't step down when no other replica is about to pick up the leadership, it's safe. -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 22:17:16 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2973 Normalize table names for Ranger
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15018 ) Change subject: KUDU-2973 Normalize table names for Ranger .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/15018/4/src/kudu/common/table_util.cc File src/kudu/common/table_util.cc: http://gerrit.cloudera.org:8080/#/c/15018/4/src/kudu/common/table_util.cc@45 PS4, Line 45:"ABCDEFGHIJKLMNOPQRSTUVWXYZ" :"0123456789" :"_/"); Nit: indent by one more char it looks like. http://gerrit.cloudera.org:8080/#/c/15018/4/src/kudu/common/table_util.cc@57 PS4, Line 57: return -2; If this is a magic value, return an enum and pass the separator out by argument. -- To view, visit http://gerrit.cloudera.org:8080/15018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11431ff5bc75540edff56ef3d4ad384fa37d33d5 Gerrit-Change-Number: 15018 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 21:45:39 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the RAT report
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15038 ) Change subject: [build] Fix the RAT report .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b Gerrit-Change-Number: 15038 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 21:42:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 21:42:28 + Gerrit-HasComments: No
[kudu-CR] [python] Fix Python 3 syntax issues
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 21:42:10 + Gerrit-HasComments: No
[kudu-CR] [build] Fix the RAT report
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15038 ) Change subject: [build] Fix the RAT report .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b Gerrit-Change-Number: 15038 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 21:39:21 + Gerrit-HasComments: No
[kudu-CR] [python] Fix Python 3 syntax issues
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 20:52:44 + Gerrit-HasComments: No
[kudu-CR] [python] Fix Python 3 syntax issues
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/15037 )
Change subject: [python] Fix Python 3 syntax issues
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/parse_test_failure.py
File build-support/parse_test_failure.py:
http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/parse_test_failure.py@292
PS1, Line 292: open(os.path.join(self._TEST_DIR, child)).read()
> Nit: In this case I think we are relying on Python's garbage collector to c
Done
http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/release/rat_exclude_files.txt
File build-support/release/rat_exclude_files.txt:
PS1:
> If you're feeling charitable you can break this change out into a separate
Done
PS1:
> Good catch. I meant to. This was just found while testing some of the Pytho
Done
http://gerrit.cloudera.org:8080/#/c/15037/1/src/kudu/scripts/max_skew_estimate.py
File src/kudu/scripts/max_skew_estimate.py:
http://gerrit.cloudera.org:8080/#/c/15037/1/src/kudu/scripts/max_skew_estimate.py@87
PS1, Line 87: print("%02d percentile: %d" % (p, percentile(skews, p)))
> Nit: I think this print formatting is considered old-style and newer style
Done
--
To view, visit http://gerrit.cloudera.org:8080/15037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd
Gerrit-Change-Number: 15037
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Jan 2020 20:34:29 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15012 to look at the new patch set (#7). Change subject: KUDU-3011 p5: transfer leadership when quiescing .. KUDU-3011 p5: transfer leadership when quiescing This amends the behavior of quiescing such that when a tablet server is quiescing, it will transfer leadership to a caught-up follower as soon as it can. While in this state, unlike while in a graceful stepdown period, the tablet can still be written to, as to not obstruct on-going workloads. Tests are added to exercise: - The basic behavior: even without injecting any errors that might cause elections, a quiescing leader will relinquish leadership. - The behavior when there are followers being caught up. In such cases, the leader won't immediately relinquish leadership -- instead, it will wait for the followers to catch up before stepping down. - The behavior when being written to. The fact that a leader is quiescing shouldn't affect its ability to be written to. - The behavior of the PeerMessageQueue when responding to various peer responses. I also removed some election-causing injection in a couple existing tests that was previously required to transfer leadership while quiescing. Note: right now, if all tablet servers are quiescing while there is a write workload on-going, a large number of StartElection requests will be sent from the leaders to the followers. A follow-up patch will address this. Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 --- M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 308 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15012/7 -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2973 Normalize table names for Ranger
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15018 ) Change subject: KUDU-2973 Normalize table names for Ranger .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/15018/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15018/2/src/kudu/master/catalog_manager.cc@5168 PS2, Line 5168: > what happens if the user has two tables, 'TableA' and 'tablea' and want to > enforce different policies on both that can't happen as we call NormalizeTableNames. > it is possible to use the non-normalized table name when retrieving Ranger > policies and leave the NormalizeTableName for HMS sync untouched? the problem is if they later enable HMS sync, permissions between access through HMS and direct access would be inconsistent. -- To view, visit http://gerrit.cloudera.org:8080/15018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11431ff5bc75540edff56ef3d4ad384fa37d33d5 Gerrit-Change-Number: 15018 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 20:32:55 + Gerrit-HasComments: Yes
[kudu-CR] [python] Fix Python 3 syntax issues
Hello Kudu Jenkins, Adar Dembo, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15037 to look at the new patch set (#2). Change subject: [python] Fix Python 3 syntax issues .. [python] Fix Python 3 syntax issues This patch fixes many of the Python 3 incompatibilities in the various Kudu Python scripts while still maintiaining Python 2 compatibility. Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd --- M build-support/build_source_release.py M build-support/check_compatibility.py M build-support/clang_tidy_gerrit.py M build-support/dist_test.py M build-support/iwyu.py M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py M build-support/parse_test_failure.py M build-support/push_to_asf.py M build-support/release/check-rat-report.py M build-support/run_dist_test.py M build-support/test_result_server.py M examples/python/graphite-kudu/kudu/kudu_graphite.py M src/kudu/benchmarks/wal_hiccup-parser.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/graph-metrics.py M src/kudu/scripts/max_skew_estimate.py M src/kudu/scripts/parse_metrics_log.py M src/kudu/scripts/write-jobs-stats-to-mysql.py 19 files changed, 63 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/15037/2 -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the RAT report
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15038 Change subject: [build] Fix the RAT report .. [build] Fix the RAT report The patch fixes the RAT report by excluding license checks for the testdata files. Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b --- M build-support/release/rat_exclude_files.txt 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15038/1 -- To view, visit http://gerrit.cloudera.org:8080/15038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I453f6fcd6e1dd29fa7fc2485db9e7a13879d6f9b Gerrit-Change-Number: 15038 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2973 Normalize table names for Ranger
Hello Kudu Jenkins, Anonymous Coward (314), Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15018 to look at the new patch set (#4). Change subject: KUDU-2973 Normalize table names for Ranger .. KUDU-2973 Normalize table names for Ranger The table names are "normalized" if HMS integration is enabled so that they can be synchronized to HMS. Hive's table/database identifiers are pretty restrictive (alphanumerical ASCII characters + underscore and forward slash) and it's case-insensitive. As Kudu doesn't support databases it imposes a further restriction: the table identifiers to be synchronized with HMS have to contain exactly one "." character which separates the database name from the table name in HMS. Ranger's restrictions regarding identifiers are more lax, but we still need to follow the stricter requirements as if a user turns on HMS integration after Ranger authorization, the identifiers still need to be valid and they must match in both places. The reason for this is that HMS and its clients (Impala, Hive, Spark, etc) also perform authorization so if a user is permitted to write to a table in Ranger, they could still be denied when trying to access the same table through Impala for example if the identifiers in HMS and Ranger didn't match up. Change-Id: I11431ff5bc75540edff56ef3d4ad384fa37d33d5 --- M CMakeLists.txt M src/kudu/common/table_util-test.cc M src/kudu/common/table_util.cc M src/kudu/common/table_util.h M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/ranger/CMakeLists.txt A src/kudu/ranger/ranger_client.cc A src/kudu/ranger/ranger_client.h 9 files changed, 189 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/15018/4 -- To view, visit http://gerrit.cloudera.org:8080/15018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I11431ff5bc75540edff56ef3d4ad384fa37d33d5 Gerrit-Change-Number: 15018 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 20:07:35 + Gerrit-HasComments: No
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/consensus_queue.cc@1144 PS1, Line 1144: > extra semi-colon Done http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/raft_consensus.cc@1644 PS1, Line 1644: PREDICT_TRUE(server_c > You can wrap this in PREDICT_TRUE() if you want. Done -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 20:04:20 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15035 to look at the new patch set (#2). Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. KUDU-3011 p6: don't transfer leadership to quiescing followers When a tablet server is quiescing, any followers hosted on it should not be considered candidates to be the leader's successor. A quiescing follower already would never become a leader because it would reject the StartElection request immediately. This patch improves upon this by nipping such requests in the bud. Followers will now send along with their ConsensusResponses whether or not they're quiescing, and if they are, the leader will know not to transfer leadership to it. I considered another approach to reducing the number of fruitless RPCs sent -- simply throttling the interval with which a leader can send StartElection requests. I opted to go with the current approach since it is more complete with regards to preventing extraneous StartElection requests. Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 --- M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 49 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/15035/2 -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 20:03:08 + Gerrit-HasComments: No
[kudu-CR] [java] support resolve one master address to multiple addresses
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15036 )
Change subject: [java] support resolve one master address to multiple addresses
..
Patch Set 1:
(5 comments)
Will you be making an equivalent change to the C++ client?
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java:
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@144
PS1, Line 144: Deferred d =
Deferred.fromError(new NonRecoverableException(statusIOE));
Too long, wrap.
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@182
PS1, Line 182:
whitespace
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@203
PS1, Line 203: InetAddress inetAddress =
NetUtil.getInetAddress(hostAndPort.getHost());
> Curious similarly why not fetch all addresses of each master host like abov
+1; why should the "resolve all" behavior only happen for one master?
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java
File java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java:
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java@57
PS1, Line 57: InetAddress []
Javadoc the method. and change type to InetAddress[]
http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java@136
PS1, Line 136: public static InetAddress[] getAllInetAddresses(final String
host) {
Can the implementation be consolidated somewhat with getInetAddress? It's a
shame to have to duplicate those branches and log messages.
--
To view, visit http://gerrit.cloudera.org:8080/15036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie242bc1331596902fa16f1c6d1b439d78b73977a
Gerrit-Change-Number: 15036
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Jan 2020 20:01:25 +
Gerrit-HasComments: Yes
[kudu-CR] [java] support resolve one master address to multiple addresses
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15036 ) Change subject: [java] support resolve one master address to multiple addresses .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java File java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java: http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@57 PS1, Line 57: private List> masterAddrsWithNames; Based on the usage, doesn't seem like this needs to be a member variable. Local variable in method connectToMasters() would suffice, no? http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@203 PS1, Line 203: InetAddress inetAddress = NetUtil.getInetAddress(hostAndPort.getHost()); Curious similarly why not fetch all addresses of each master host like above? http://gerrit.cloudera.org:8080/#/c/15036/1/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java@214 PS1, Line 214: Deferred d; Nit: Better to combine the definition and assignment of "d" to line 219 below. -- To view, visit http://gerrit.cloudera.org:8080/15036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie242bc1331596902fa16f1c6d1b439d78b73977a Gerrit-Change-Number: 15036 Gerrit-PatchSet: 1 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:57:59 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc@277 PS5, Line 277: RaftConfigPB config = BuildRaftConfigPBForTests(/*num_voters*/2); > Not actually used? Ah yeah, we need to implement NotifyCommitIndex() since it gets called. I'll update this so everything else is a no-op. http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc@278 PS5, Line 278: queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, config); > Should generally prefer deque to list. Done http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/integration-tests/tablet_server_quiescing-itest.cc File src/kudu/integration-tests/tablet_server_quiescing-itest.cc: http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@525 PS5, Line 525: u > u Done -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:56:58 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15012 to look at the new patch set (#6). Change subject: KUDU-3011 p5: transfer leadership when quiescing .. KUDU-3011 p5: transfer leadership when quiescing This amends the behavior of quiescing such that when a tablet server is quiescing, it will transfer leadership to a caught-up follower as soon as it can. While in this state, unlike while in a graceful stepdown period, the tablet can still be written to, as to not obstruct on-going workloads. Tests are added to exercise: - The basic behavior: even without injecting any errors that might cause elections, a quiescing leader will relinquish leadership. - The behavior when there are followers being caught up. In such cases, the leader won't immediately relinquish leadership -- instead, it will wait for the followers to catch up before stepping down. - The behavior when being written to. The fact that a leader is quiescing shouldn't affect its ability to be written to. - The behavior of the PeerMessageQueue when responding to various peer responses. I also removed some election-causing injection in a couple existing tests that was previously required to transfer leadership while quiescing. Note: right now, if all tablet servers are quiescing while there is a write workload on-going, a large number of StartElection requests will be sent from the leaders to the followers. A follow-up patch will address this. Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 --- M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc 6 files changed, 307 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15012/6 -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3011 p6: don't transfer leadership to quiescing followers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15035 ) Change subject: KUDU-3011 p6: don't transfer leadership to quiescing followers .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/consensus_queue.cc@1144 PS1, Line 1144: ; extra semi-colon http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/15035/1/src/kudu/consensus/raft_consensus.cc@1644 PS1, Line 1644: server_ctx_.quiescing You can wrap this in PREDICT_TRUE() if you want. -- To view, visit http://gerrit.cloudera.org:8080/15035 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74ec79d0bc4dbe42fce0cca2e001cd0b369cd066 Gerrit-Change-Number: 15035 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:52:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3011 p5: transfer leadership when quiescing
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15012 ) Change subject: KUDU-3011 p5: transfer leadership when quiescing .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc@277 PS5, Line 277: list commit_indexes_; Not actually used? http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/consensus/consensus_queue-test.cc@278 PS5, Line 278: list peers_to_start_election_; Should generally prefer deque to list. http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/integration-tests/tablet_server_quiescing-itest.cc File src/kudu/integration-tests/tablet_server_quiescing-itest.cc: http://gerrit.cloudera.org:8080/#/c/15012/5/src/kudu/integration-tests/tablet_server_quiescing-itest.cc@525 PS5, Line 525: U u -- To view, visit http://gerrit.cloudera.org:8080/15012 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf0716f5c9455f83ff5f6f601b0f5042f77d078 Gerrit-Change-Number: 15012 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:48:39 + Gerrit-HasComments: Yes
[kudu-CR] [python] Fix Python 3 syntax issues
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/release/rat_exclude_files.txt File build-support/release/rat_exclude_files.txt: PS1: > If you're feeling charitable you can break this change out into a separate Good catch. I meant to. This was just found while testing some of the Python scripts. -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:44:16 + Gerrit-HasComments: Yes
[kudu-CR] [metrics] Fix bugs when metrics do merge
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15014 ) Change subject: [metrics] Fix bugs when metrics do merge .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15014/2/src/kudu/master/table_metrics.cc File src/kudu/master/table_metrics.cc: http://gerrit.cloudera.org:8080/#/c/15014/2/src/kudu/master/table_metrics.cc@23 PS2, Line 23: METRIC_DECLARE_gauge_size(merged_entities_count_of_table); > merged_entities_count_of_xx is defined in metrics.h:L266, in METRIC_DEFINE_ Ah I forgot about that. Thanks for the reminder. -- To view, visit http://gerrit.cloudera.org:8080/15014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 Gerrit-Change-Number: 15014 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Wed, 15 Jan 2020 19:39:59 + Gerrit-HasComments: Yes
[kudu-CR] [metrics] Fix bugs when metrics do merge
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15014 ) Change subject: [metrics] Fix bugs when metrics do merge .. [metrics] Fix bugs when metrics do merge 1. METRIC_merged_entities_count_of_server should never retire. 2. Add METRIC_merged_entities_count_of_tablet/table's ref counter to avoid retiring before its bounded MetricEntity destroyed. 3. Metric's modification epoch should be updated when AtomicGauge set_value(). 4. Merge should abort if metrics are invalid for merging, FunctionGauge included. Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 Reviewed-on: http://gerrit.cloudera.org:8080/15014 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/master/catalog_manager.cc M src/kudu/master/table_metrics.cc M src/kudu/master/table_metrics.h M src/kudu/server/server_base.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet_metrics.cc M src/kudu/tablet/tablet_metrics.h M src/kudu/util/metrics.h 8 files changed, 24 insertions(+), 8 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 Gerrit-Change-Number: 15014 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [python] Fix Python 3 syntax issues
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15037 ) Change subject: [python] Fix Python 3 syntax issues .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/release/rat_exclude_files.txt File build-support/release/rat_exclude_files.txt: PS1: If you're feeling charitable you can break this change out into a separate patch as it really has nothing to do with py3 compat. -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jan 2020 19:39:36 + Gerrit-HasComments: Yes
[kudu-CR] Integrate BlockBloomFilter with ColumnPredicate on server side
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 3: Adding original authors/reviewers of KUDU-2483. -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 15 Jan 2020 18:33:59 + Gerrit-HasComments: No
[kudu-CR] [python] Fix Python 3 syntax issues
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/15037 )
Change subject: [python] Fix Python 3 syntax issues
..
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/parse_test_failure.py
File build-support/parse_test_failure.py:
http://gerrit.cloudera.org:8080/#/c/15037/1/build-support/parse_test_failure.py@292
PS1, Line 292: open(os.path.join(self._TEST_DIR, child)).read()
Nit: In this case I think we are relying on Python's garbage collector to close
the file.
Would it be better to explicitly close the file using with clause/finally
close() or this is not long running enough to cause resource leak issue?
Same for couple of other places below.
http://gerrit.cloudera.org:8080/#/c/15037/1/src/kudu/scripts/max_skew_estimate.py
File src/kudu/scripts/max_skew_estimate.py:
http://gerrit.cloudera.org:8080/#/c/15037/1/src/kudu/scripts/max_skew_estimate.py@87
PS1, Line 87: print("%02d percentile: %d" % (p, percentile(skews, p)))
Nit: I think this print formatting is considered old-style and newer style is
to use "".format(). Something to consider changing while making the
compatibility change.
--
To view, visit http://gerrit.cloudera.org:8080/15037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd
Gerrit-Change-Number: 15037
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Jan 2020 18:06:09 +
Gerrit-HasComments: Yes
[kudu-CR] [python] Fix Python 3 syntax issues
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15037 Change subject: [python] Fix Python 3 syntax issues .. [python] Fix Python 3 syntax issues This patch fixes many of the Python 3 incompatibilities in the various Kudu Python scripts while still maintiaining Python 2 compatibility. Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd --- M build-support/build_source_release.py M build-support/check_compatibility.py M build-support/clang_tidy_gerrit.py M build-support/dist_test.py M build-support/iwyu.py M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py M build-support/parse_test_failure.py M build-support/push_to_asf.py M build-support/release/check-rat-report.py M build-support/release/rat_exclude_files.txt M build-support/run_dist_test.py M build-support/test_result_server.py M examples/python/graphite-kudu/kudu/kudu_graphite.py M src/kudu/benchmarks/wal_hiccup-parser.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/graph-metrics.py M src/kudu/scripts/max_skew_estimate.py M src/kudu/scripts/parse_metrics_log.py M src/kudu/scripts/write-jobs-stats-to-mysql.py 20 files changed, 61 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/15037/1 -- To view, visit http://gerrit.cloudera.org:8080/15037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5d5e06ea6299032d84a76440afb42a70fd461abd Gerrit-Change-Number: 15037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [java] support resolve one master address to multiple addresses
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15036 Change subject: [java] support resolve one master address to multiple addresses .. [java] support resolve one master address to multiple addresses The master addresses of kudu client are fixed during runtime, we need to restart the client if we want to change the master addresses. In some network settings we can map a domain name to multiple ip addresses, if we configure the client with such a domain name as a 'master address', resolve it and try to connect each ip address, we can easily change the master addresses by network settings modifications. Change-Id: Ie242bc1331596902fa16f1c6d1b439d78b73977a --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java 3 files changed, 87 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/15036/1 -- To view, visit http://gerrit.cloudera.org:8080/15036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie242bc1331596902fa16f1c6d1b439d78b73977a Gerrit-Change-Number: 15036 Gerrit-PatchSet: 1 Gerrit-Owner: Yifan Zhang
