[kudu-CR] [catalog manager] cache masters' addresses
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15704 ) Change subject: [catalog manager] cache masters' addresses .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9afea2d708bd3060b3d9f5b672660e1d2dca910 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 10 Apr 2020 22:30:31 + Gerrit-HasComments: No
[kudu-CR] [catalog manager] cache masters' addresses
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15704 ) Change subject: [catalog manager] cache masters' addresses .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15704/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15704/1//COMMIT_MSG@35 PS1, Line 35: This patch introduces caching of the master host ports, so there is Worth noting that if/when we implement dynamic master change config, we'll need to refresh the cached state on change config. -- To view, visit http://gerrit.cloudera.org:8080/15704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9afea2d708bd3060b3d9f5b672660e1d2dca910 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 10 Apr 2020 22:22:33 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] reduce contention in ScopedLeaderSharedLock
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15698 ) Change subject: [catalog_manager] reduce contention in ScopedLeaderSharedLock .. Patch Set 1: (1 comment) Good catch! http://gerrit.cloudera.org:8080/#/c/15698/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15698/1/src/kudu/master/catalog_manager.cc@5319 PS1, Line 5319: int64_t leader_ready_term = -1; Don't need to initialize? -- To view, visit http://gerrit.cloudera.org:8080/15698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3b2e6866a8a0d5bda9e2b1f01e0668427de60868 Gerrit-Change-Number: 15698 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 09 Apr 2020 23:01:41 + Gerrit-HasComments: Yes
[kudu-CR] WIP: KUDU-3097 whether master keep record could be configurable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15616 ) Change subject: WIP: KUDU-3097 whether master keep record could be configurable .. Patch Set 4: > But it would cause some inconsistent between table and tablets. Cause tablet > should be keep in memory forever in order to not overlap with previously > deleted table. > E.g table t1 with tablet uuid 1,2,3 > Then I deleted it, the tablet uuid 1,2,3 shouldn't be bind to other new table > t2,t3 ... > And also I have to handle other things like this, table not exists but tablet > still in tablet_map_. > > So if I want to implement this behave, I should handle things like this. > > Is my understand about this right? Table and tablet UUIDs are expected to be universally unique, so you don't need to preserve them in-memory just to avoid reusing them in the future. There shouldn't ever be any collisions. Furthermore, table and tablet metadata should always be consistent with one another: either both should be preserved, or both should be deleted. It'd be problematic to preserve tablet metadata while discarding table metadata. Hope this helps! -- To view, visit http://gerrit.cloudera.org:8080/15616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1088080d706edadeed9d93f54704c7f4bfe2b6a2 Gerrit-Change-Number: 15616 Gerrit-PatchSet: 4 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: wangning <1994wangn...@gmail.com> Gerrit-Comment-Date: Wed, 08 Apr 2020 20:29:42 + Gerrit-HasComments: No
[kudu-CR] [master] KUDU-2798 fix logging on deleted TSK entries
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15657 ) Change subject: [master] KUDU-2798 fix logging on deleted TSK entries .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15657 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I98c7ba7fd2277fff1176eca51d59404deebe38c4 Gerrit-Change-Number: 15657 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 07 Apr 2020 01:04:02 + Gerrit-HasComments: No
[kudu-CR] util: remove duplicate results from DNS resolution
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15665 ) Change subject: util: remove duplicate results from DNS resolution .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15665/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15665/1//COMMIT_MSG@13 PS1, Line 13: $ kudu table list localhost : W0404 20:35:05.511526 31378 client-internal.cc:597] Specified master : server address 'localhost' resolved to multiple IPs. Using : 127.0.0.1:7051 I've definitely seen this before and always wondered what caused it. Is it due to poor DNS configuration? A bug in glibc? Something else entirely? -- To view, visit http://gerrit.cloudera.org:8080/15665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d9b9f9839a899d8022f5ac6496555ff84583192 Gerrit-Change-Number: 15665 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Apr 2020 22:49:08 + Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-2798 fix logging on deleted TSK entries
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15657 )
Change subject: [master] KUDU-2798 fix logging on deleted TSK entries
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15657/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15657/2/src/kudu/master/catalog_manager.cc@4592
PS2, Line 4592: for (const auto& entry_id : entry_ids) {
: msg += Substitute(" $0",
SysCatalogTable::TskEntryIdToSeqNumber(entry_id));
: }
JoinMapped() is a pretty handy way to do loop transformations like these.
http://gerrit.cloudera.org:8080/#/c/15657/2/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/15657/2/src/kudu/master/sys_catalog.cc@636
PS2, Line 636: false, nullptr,
Nit: annotate with comments what these mean?
--
To view, visit http://gerrit.cloudera.org:8080/15657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98c7ba7fd2277fff1176eca51d59404deebe38c4
Gerrit-Change-Number: 15657
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Volodymyr Verovkin
Gerrit-Comment-Date: Mon, 06 Apr 2020 22:37:19 +
Gerrit-HasComments: Yes
[kudu-CR] bitshuffle: check for a multiple-of-8-bytes invariant in Finish
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15652 ) Change subject: bitshuffle: check for a multiple-of-8-bytes invariant in Finish .. bitshuffle: check for a multiple-of-8-bytes invariant in Finish Change-Id: I447a78739f19ee5a7dfd8935bdfb273a5b71369a Reviewed-on: http://gerrit.cloudera.org:8080/15652 Reviewed-by: Bankim Bhavsar Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Bankim Bhavsar: Looks good to me, but someone else must approve Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15652 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I447a78739f19ee5a7dfd8935bdfb273a5b71369a Gerrit-Change-Number: 15652 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: allow overwriting of the log4j2 properties file
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15650 ) Change subject: ranger: allow overwriting of the log4j2 properties file .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15650 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a06f8a1b3328cfd4029295527b5ba61a03efbfa Gerrit-Change-Number: 15650 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 22:08:31 + Gerrit-HasComments: No
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15647 ) Change subject: bitshuffle: stop using uninitialized data as padding .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15647/2/src/kudu/cfile/bshuf_block.h File src/kudu/cfile/bshuf_block.h: http://gerrit.cloudera.org:8080/#/c/15647/2/src/kudu/cfile/bshuf_block.h@186 PS2, Line 186: > With this fix the data_ buffer will indeed be multiple of 8 unlike earlier kHeaderSize is 20. I presume you mean data_ will be a multiple of 8 _after the padding is added_? As I already merged this patch, I've added the DCHECK in a new commit: http://gerrit.cloudera.org:8080/15652 -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 04 Apr 2020 21:39:52 + Gerrit-HasComments: Yes
[kudu-CR] bitshuffle: check for a multiple-of-8-bytes invariant in Finish
Hello Andrew Wong, Bankim Bhavsar, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15652 to review the following change. Change subject: bitshuffle: check for a multiple-of-8-bytes invariant in Finish .. bitshuffle: check for a multiple-of-8-bytes invariant in Finish Change-Id: I447a78739f19ee5a7dfd8935bdfb273a5b71369a --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/15652/1 -- To view, visit http://gerrit.cloudera.org:8080/15652 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I447a78739f19ee5a7dfd8935bdfb273a5b71369a Gerrit-Change-Number: 15652 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar
[kudu-CR] ranger: allow overwriting of the log4j2 properties file
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15650 ) Change subject: ranger: allow overwriting of the log4j2 properties file .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15650/2/src/kudu/ranger/ranger_client-test.cc File src/kudu/ranger/ranger_client-test.cc: http://gerrit.cloudera.org:8080/#/c/15650/2/src/kudu/ranger/ranger_client-test.cc@371 PS2, Line 371: RETURN_NOT_OK(ListFilesInDir(env, dir, )); How about using Env::Glob for this and the regexp stuff? -- To view, visit http://gerrit.cloudera.org:8080/15650 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a06f8a1b3328cfd4029295527b5ba61a03efbfa Gerrit-Change-Number: 15650 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 21:10:00 + Gerrit-HasComments: Yes
[kudu-CR] WIP: MSAN support
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15649 to review the following change. Change subject: WIP: MSAN support .. WIP: MSAN support WIP because I can't seem to get suppression of third party code to work, and this is a complete non-starter otherwise (i.e. too many unsafe accesses in libunwind, libsasl, and openssl for starters). Change-Id: Iff02896fa7e3adf2a920214d8bc3d040226ebe6f --- M CMakeLists.txt M cmake_modules/KuduLinker.cmake M src/kudu/client/CMakeLists.txt M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/gutil/dynamic_annotations.h M src/kudu/gutil/port.h M src/kudu/util/CMakeLists.txt M src/kudu/util/bitmap-test.cc M src/kudu/util/crc-test.cc M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/pb_util-test.cc M src/kudu/util/sanitizer_options.cc M thirdparty/build-definitions.sh M thirdparty/build-if-necessary.sh M thirdparty/build-thirdparty.sh M thirdparty/vars.sh 18 files changed, 339 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/15649/1 -- To view, visit http://gerrit.cloudera.org:8080/15649 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iff02896fa7e3adf2a920214d8bc3d040226ebe6f Gerrit-Change-Number: 15649 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15647 ) Change subject: bitshuffle: stop using uninitialized data as padding .. bitshuffle: stop using uninitialized data as padding The resize() incorrectly accounted for a block header that was never actually written to data_. The result was that added padding was actually kHeaderSize bytes "to the right", and the call to compress_lz4() read uninitialized data from this part of data_ rather than the added padding. What's the effect? Up to padding_bytes of uninitialized data gets bitshuffled, compressed, and written to the block. At read time, it is decompressed, debitshuffled, but ultimately ignored, as it was expected to be just padded zeroes. I observed this in cfile-test's TestMetadata built with MSAN instrumentation, but oddly enough not in any other test in cfile-test, even though others use bitshuffle and padding. Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Reviewed-on: http://gerrit.cloudera.org:8080/15647 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:48:19 + Gerrit-HasComments: No
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. remove last vestiges of boost::bind, boost::function, and std::bind With this patch, all Kudu functors use std::function, and all capturing or binding is done via C++ lambdas. The lack of support for capture-via-move rears its ugly head in raft_consensus.cc, but I verified that this only leads to inefficiency, not incorrectness (i.e. lifecycle issues). C++14 (or 17) support can't come soon enough! Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Reviewed-on: http://gerrit.cloudera.org:8080/15639 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.h M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client-unittest.cc M src/kudu/client/client.cc M src/kudu/client/master_proxy_rpc.cc M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/session-internal.cc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/reactor-test.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/response_callback.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc.cc M src/kudu/rpc/rpc.h M src/kudu/rpc/rpc_stub-test.cc M src/kudu/security/init.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/server_base.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/server/webserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_path_handlers.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/url-coding.cc M src/kudu/util/web_callback_registry.h 57 files changed, 522 insertions(+), 550 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 9: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296 PS8, Line 296: K(WriteStringToFileSync( > That doesn't seem to work (it's not honored by log4j2 at least). Leaving th Oh I misunderstood; I thought this was "our" parameter. -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Wong 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:19:17 + Gerrit-HasComments: Yes
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Hello Andrew Wong, Bankim Bhavsar, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15647 to review the following change. Change subject: bitshuffle: stop using uninitialized data as padding .. bitshuffle: stop using uninitialized data as padding The resize() incorrectly accounted for a block header that was never actually written to data_. The result was that added padding was actually kHeaderSize bytes "to the right", and the call to compress_lz4() read uninitialized data from this part of data_ rather than the added padding. What's the effect? Up to padding_bytes of uninitialized data gets bitshuffled, compressed, and written to the block. At read time, it is decompressed, debitshuffled, but ultimately ignored, as it was expected to be just padded zeroes. I observed this in cfile-test's TestMetadata built with MSAN instrumentation, but oddly enough not in any other test in cfile-test, even though others use bitshuffle and padding. Change-Id: I25eba027ba356774173b2313c68436d7baddaddc --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/15647/1 -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Todd Lipcon
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
..
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@13
PS8, Line 13: This means a file will not be regenerated
: if on has already been generated
One downside to this approach is that we can't automatically update
log4j2.properties in subsequent releases.
I think that's actually more important than providing a mechanism for admins to
supply their own log4j2.properties.
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@14
PS8, Line 14: on
one
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@17
PS8, Line 17: directory
Nit: the directory
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@255
PS8, Line 255: if (!env->FileExists(log_conf_dir)) {
: RETURN_NOT_OK(env->CreateDir(log_conf_dir));
: }
I don't think we should be creating FLAGS_log_dir. Maybe condition this on
whether we're using FLAGS_ranger_log_config_dir? Or even then, force admins to
create it?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: -Dlog4j2.configurationFile
Nit: maybe -Dlog4j2.configuration.filename?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/subprocess/subprocess_proxy.cc
File src/kudu/subprocess/subprocess_proxy.cc:
PS8:
We're doing more and more of this "template composition", and it's getting
increasingly complicated. At some point we're better off using a real
templating system, which should be able to handle composition based on a
variety of conditions (loops, if/else, etc.).
We've got mustache in the codebase and I think it'd be a reasonable fit.
Currently mustache expects template files to live on disk, but there's no
reason we can't modify it to reference them from memory.
Doesn't apply to this patch, but something to think about.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 21:30:54 +
Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > nit: while you are here, maybe add a comment that the signature of this fun I think that's already implied in the existing comment, no? "libev callback for handling ...". http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > ditto See above. -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo 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: Fri, 03 Apr 2020 21:12:04 + Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15639 to review the following change. Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. remove last vestiges of boost::bind, boost::function, and std::bind With this patch, all Kudu functors use std::function, and all capturing or binding is done via C++ lambdas. The lack of support for capture-via-move rears its ugly head in raft_consensus.cc, but I verified that this only leads to inefficiency, not incorrectness (i.e. lifecycle issues). C++14 (or 17) support can't come soon enough! Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.h M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client-unittest.cc M src/kudu/client/client.cc M src/kudu/client/master_proxy_rpc.cc M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/session-internal.cc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/reactor-test.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/response_callback.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc.cc M src/kudu/rpc/rpc.h M src/kudu/rpc/rpc_stub-test.cc M src/kudu/security/init.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/server_base.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/server/webserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_path_handlers.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/url-coding.cc M src/kudu/util/web_callback_registry.h 57 files changed, 522 insertions(+), 550 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/15639/1 -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] KUDU-3081 Add Kerberos support to MiniRanger
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15601 ) Change subject: KUDU-3081 Add Kerberos support to MiniRanger .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32118780ad912791fe5e371004345428b6459549 Gerrit-Change-Number: 15601 Gerrit-PatchSet: 14 Gerrit-Owner: Attila Bukor 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 Apr 2020 21:26:22 + Gerrit-HasComments: No
[kudu-CR] [ranger] Use unique loopback for MiniRanger
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15625 ) Change subject: [ranger] Use unique loopback for MiniRanger .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15625 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8833bb149fb2410f4f140b8fb67b1039a2f90d0 Gerrit-Change-Number: 15625 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 Apr 2020 21:24:39 + Gerrit-HasComments: No
[kudu-CR] [postgres] Deflake MiniPostgres tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15629 ) Change subject: [postgres] Deflake MiniPostgres tests .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5106b3e2aeb9dabad9a7ee0b17059c1df2042bb4 Gerrit-Change-Number: 15629 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 02 Apr 2020 21:24:13 + Gerrit-HasComments: No
[kudu-CR] [subprocess] Fix shutdown behavior
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15615 ) Change subject: [subprocess] Fix shutdown behavior .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia84bff891eaff667d7b22e89c66f24980ad76d70 Gerrit-Change-Number: 15615 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor 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: Thu, 02 Apr 2020 21:23:16 + Gerrit-HasComments: No
[kudu-CR] [build] Fix boost compilation on latest macOS Catalina
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15632 ) Change subject: [build] Fix boost compilation on latest macOS Catalina .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bb488dcadf215be708ae1b1e89ddd77bd30074 Gerrit-Change-Number: 15632 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Apr 2020 22:11:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-3081 Add Kerberos support to MiniRanger
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15601 )
Change subject: KUDU-3081 Add Kerberos support to MiniRanger
..
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15601/10/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15601/10/src/kudu/ranger/ranger_client.cc@196
PS10, Line 196: ret.emplace_back(Substitute("-Djava.security.krb5.conf=$0",
krb5_config));
> Nit: given we don't have any getters for this, maybe just call GetKrb5Confi
+1
--
To view, visit http://gerrit.cloudera.org:8080/15601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32118780ad912791fe5e371004345428b6459549
Gerrit-Change-Number: 15601
Gerrit-PatchSet: 10
Gerrit-Owner: Attila Bukor
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 01 Apr 2020 22:09:11 +
Gerrit-HasComments: Yes
[kudu-CR] [postgres] Deflake MiniPostgres tests
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15629 )
Change subject: [postgres] Deflake MiniPostgres tests
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15629/4/src/kudu/postgres/mini_postgres.cc
File src/kudu/postgres/mini_postgres.cc:
http://gerrit.cloudera.org:8080/#/c/15629/4/src/kudu/postgres/mini_postgres.cc@168
PS4, Line 168: return Status::RemoteError("Couldn't connect to Postgres");
Could you preserve the first (or last) result of WaitAndCheckExitCode(), and
return it here, assuming it's worth something?
--
To view, visit http://gerrit.cloudera.org:8080/15629
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5106b3e2aeb9dabad9a7ee0b17059c1df2042bb4
Gerrit-Change-Number: 15629
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor
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, 01 Apr 2020 19:26:50 +
Gerrit-HasComments: Yes
[kudu-CR] [ranger] Use unique loopback for MiniRanger
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15625 ) Change subject: [ranger] Use unique loopback for MiniRanger .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15625 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8833bb149fb2410f4f140b8fb67b1039a2f90d0 Gerrit-Change-Number: 15625 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Bukor 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 01 Apr 2020 19:24:53 + Gerrit-HasComments: No
[kudu-CR] [build] Fix boost compilation on latest macOS Catalina update
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15632 ) Change subject: [build] Fix boost compilation on latest macOS Catalina update .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15632/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15632/1//COMMIT_MSG@13 PS1, Line 13: See https://github.com/zcash/zcash/pull/4334 for additional context. Could you actually adopt the solution in https://github.com/boostorg/build/pull/560? Seems more authoritative, and more likely to make it into boost proper. -- To view, visit http://gerrit.cloudera.org:8080/15632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bb488dcadf215be708ae1b1e89ddd77bd30074 Gerrit-Change-Number: 15632 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Apr 2020 19:19:48 + Gerrit-HasComments: Yes
[kudu-CR] [subprocess] Fix shutdown behavior
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15615 ) Change subject: [subprocess] Fix shutdown behavior .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/15615/3/java/config/spotbugs/excludeFilter.xml File java/config/spotbugs/excludeFilter.xml: http://gerrit.cloudera.org:8080/#/c/15615/3/java/config/spotbugs/excludeFilter.xml@311 PS3, Line 311: all threads stop -- > Done FWIW I think the existing alignment was consistent with the existing file. See L220 for example. -- To view, visit http://gerrit.cloudera.org:8080/15615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia84bff891eaff667d7b22e89c66f24980ad76d70 Gerrit-Change-Number: 15615 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Bukor 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, 01 Apr 2020 19:14:15 + Gerrit-HasComments: Yes
[kudu-CR] WIP: KUDU-3097 whether master keep record could be configurable
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15616 )
Change subject: WIP: KUDU-3097 whether master keep record could be configurable
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/15616/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15616/4/src/kudu/master/catalog_manager.cc@316
PS4, Line 316: DEFINE_bool(delete_catalog_record_when_deleting_table, false,
Nit: "catalog_manager_delete_table_metadata".
http://gerrit.cloudera.org:8080/#/c/15616/4/src/kudu/master/catalog_manager.cc@317
PS4, Line 317: "Whether keep tablet record forever or just drop
when deleting table");
Nit: "Whether to remove table metadata from the system catalog when deleting
tables."
http://gerrit.cloudera.org:8080/#/c/15616/4/src/kudu/master/catalog_manager.cc@4071
PS4, Line 4071: }
Nit: should be:
} else {
http://gerrit.cloudera.org:8080/#/c/15616/4/src/kudu/master/catalog_manager.cc@4073
PS4, Line 4073: LOG(INFO) << "The tablet were dropped when deleting
table"
:"cause
FLAGS_delete_catalog_record_when_deleting_table "
:"is enabled, tablet_id is " << tablet_id;
Maybe:
"Ignoring report from unknown tablet " << tablet_id << ". It's likely that
this tablet's table was removed from the catalog; the tablet must be manually
deleted."
Also, could you try this in production? I imagine it keeps repeating the same
log over and over and you'll want to throttle it so it only logs once per
tablet ID.
--
To view, visit http://gerrit.cloudera.org:8080/15616
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1088080d706edadeed9d93f54704c7f4bfe2b6a2
Gerrit-Change-Number: 15616
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <1994wangn...@gmail.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 Apr 2020 19:03:04 +
Gerrit-HasComments: Yes
[kudu-CR] wip ranger: direct client logs to a log file
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: wip ranger: direct client logs to a log file .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15628/1//COMMIT_MSG@10 PS1, Line 10: The subprocess will use the KUDU_LOG_DIR and : KUDU_LOG_NAME system properties to output logs as appropriate. Could you add a sample listing of the log directory and the subprocess log files in it? -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Apr 2020 18:56:57 + Gerrit-HasComments: Yes
[kudu-CR] subprocess: enable log4j debug logging
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15627 ) Change subject: subprocess: enable log4j debug logging .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If22dfdb4e8ba7814c0a91ba53ec4964220a9d139 Gerrit-Change-Number: 15627 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Apr 2020 18:53:32 + Gerrit-HasComments: No
[kudu-CR] encoding-test: Clean up bitshuffle tests a little
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15043 ) Change subject: encoding-test: Clean up bitshuffle tests a little .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/15043/5/src/kudu/util/random.h File src/kudu/util/random.h: http://gerrit.cloudera.org:8080/#/c/15043/5/src/kudu/util/random.h@251 PS5, Line 251: IntType UnsignedIntType Though I don't understand why these two functions differ on signedness: both UniformN() and NextN() return unsigned integers. http://gerrit.cloudera.org:8080/#/c/15043/5/src/kudu/util/random_util.h File src/kudu/util/random_util.h: http://gerrit.cloudera.org:8080/#/c/15043/5/src/kudu/util/random_util.h@130 PS5, Line 130: template Same comment about signedness. -- 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: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Apr 2020 18:50:49 + Gerrit-HasComments: Yes
[kudu-CR] client: add support for columnar format scan
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15622 ) Change subject: client: add support for columnar format scan .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69ec4487c78b872e7b9eba5facdd44cfd6b8f4fa Gerrit-Change-Number: 15622 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Apr 2020 17:39:23 + Gerrit-HasComments: No
[kudu-CR] [ranger] Use unique loopback for MiniRanger
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15625 ) Change subject: [ranger] Use unique loopback for MiniRanger .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15625/1/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/15625/1/src/kudu/mini-cluster/external_mini_cluster.cc@339 PS1, Line 339: string host = GetBindIpForExternalServer(0); Should add some comment here about why it's OK to use the same index as for Sentry (i.e. we'll never start a cluster with both). Come to think of it, do we actually enforce that? We probably should. http://gerrit.cloudera.org:8080/#/c/15625/1/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: http://gerrit.cloudera.org:8080/#/c/15625/1/src/kudu/util/net/net_util.h@235 PS1, Line 235: // Gets a random port from the ephemeral range by binding to port 0 and letting Update to reflect the effect of 'address'. -- To view, visit http://gerrit.cloudera.org:8080/15625 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8833bb149fb2410f4f140b8fb67b1039a2f90d0 Gerrit-Change-Number: 15625 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor 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: Tue, 31 Mar 2020 23:02:11 + Gerrit-HasComments: Yes
[kudu-CR] [subprocess] Fix shutdown behavior
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15615 ) Change subject: [subprocess] Fix shutdown behavior .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia84bff891eaff667d7b22e89c66f24980ad76d70 Gerrit-Change-Number: 15615 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor 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: Tue, 31 Mar 2020 22:57:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-3081 Add Kerberos support to MiniRanger
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15601 )
Change subject: KUDU-3081 Add Kerberos support to MiniRanger
..
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/mini_ranger.cc
File src/kudu/ranger/mini_ranger.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/mini_ranger.cc@206
PS4, Line 206: string krb5_config = getenv("KRB5_CONFIG");
> Isn't it important for the KRB5_CONFIG used by MiniRanger to be the one fr
+1 to what Andrew said, and yeah, I do feel strongly about this (provided it's
not an impossible task). I already find it difficult to understand how the
various Kerberos-related environment variables affect the Kudu C++ security
code; I'd rather we avoid adding more complexity there.
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.cc@195
PS4, Line 195: string krb5_config = getenv("KRB5_CONFIG");
> Alternatively, we can expose a flag that, if set, will be used (e.g. in an
The problem is that it's much tougher to track and understand the behavior of
an env var "switch" vs. an explicit one.
Can we model this as we did kudu::thrift::ClientOptions::enable_kerberos?
1. The "library" code (RangerClient, in this case) provides an explicit setter
or options struct to enable Kerberos. This setter/option must include all the
information needed (i.e. if it needs a path to a krb5.conf file, then it
expects a string containing that path).
2. The EMC code uses this setter/options struct accordingly using information
from the MiniKdc.
3. Production code sets the setter/options using the value of a gflag. Or, in
the case of path to krb5.conf, maybe even hardcodes it to /etc/krb5.conf, as I
believe that's what the C++ security code always uses.
--
To view, visit http://gerrit.cloudera.org:8080/15601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32118780ad912791fe5e371004345428b6459549
Gerrit-Change-Number: 15601
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 31 Mar 2020 22:55:44 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3097 kudu master should delete table record when deleting table
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15616 ) Change subject: KUDU-3097 kudu master should delete table record when deleting table .. Patch Set 3: (1 comment) This needs a unit test or two, not to mention there are some existing tests that are broken. Additionally, it should be configurable via gflag (and default to off). There are several existing invariants which rely on the permanent preservation of table metadata. I've highlighted one of them. We need to find them all and understand how this change affects them. http://gerrit.cloudera.org:8080/#/c/15616/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15616/3/src/kudu/master/catalog_manager.cc@4053 PS3, Line 4053: // It'd be unsafe to ask the tserver to delete this tablet without first : // replicating something to our followers (i.e. to guarantee that we're : // the leader). For example, if we were a rogue master, we might be : // deleting a tablet created by a new master accidentally. But masters : // retain metadata for deleted tablets forever, so a tablet can only be : // truly unknown in the event of a serious misconfiguration, such as a : // tserver heartbeating to the wrong cluster. Therefore, it should be : // reasonable to ignore it and wait for an operator fix the situation. Here's one invariant that relies on the fact that table/tablet metadata is retained forever. -- To view, visit http://gerrit.cloudera.org:8080/15616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1088080d706edadeed9d93f54704c7f4bfe2b6a2 Gerrit-Change-Number: 15616 Gerrit-PatchSet: 3 Gerrit-Owner: wangning <1994wangn...@gmail.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 31 Mar 2020 20:40:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3081 Add Kerberos support to MiniRanger
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15601 )
Change subject: KUDU-3081 Add Kerberos support to MiniRanger
..
Patch Set 5:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/mini-cluster/external_mini_cluster.cc@647
PS4, Line 647:
opts.extra_flags.emplace_back("--unlock_experimental_flags");
This is already done in each Kudu process (see ExternalDaemon::StartProcess).
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/postgres/mini_postgres.cc
File src/kudu/postgres/mini_postgres.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/postgres/mini_postgres.cc@135
PS4, Line 135: config.append(Substitute("\nlisten_addresses =
'127.0.0.1'\nport = $0\n", port_));
Why was this change needed? Isn't 127.0.0.1 the default listen address? Would
be nice to doc with a comment.
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/mini_ranger.cc
File src/kudu/ranger/mini_ranger.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/mini_ranger.cc@206
PS4, Line 206: string krb5_config = getenv("KRB5_CONFIG");
Again, this is an opaque way to pass Kerberos configuration information between
cluster daemons. I'd prefer if you did what we did for Sentry/HMS: an
EnableKerberos method that takes all the (unmarshalled) information it needs as
separate arguments. Then, ExternalMiniCluster can call that method directly and
feed in information from the MiniKdc's environment variables.
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.h
File src/kudu/ranger/ranger_client.h:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.h@117
PS4, Line 117: static std::string java_path();
Nit: static (and then non-static) private member functions should be above the
data members.
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.cc@195
PS4, Line 195: string krb5_config = getenv("KRB5_CONFIG");
If this is the Kerberization switch, would rather it be an explicit setter
called on RangerClient, because otherwise it's tough to trace the relationship
between whomever consumes KRB5_CONFIG (RangerClient) and whomever sets it (the
EMC, presumably).
http://gerrit.cloudera.org:8080/#/c/15601/4/src/kudu/ranger/ranger_client.cc@198
PS4, Line 198: LOG(INFO) << krb5_config;
This was for debugging I presume, remove now?
--
To view, visit http://gerrit.cloudera.org:8080/15601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32118780ad912791fe5e371004345428b6459549
Gerrit-Change-Number: 15601
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 31 Mar 2020 20:35:49 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] Fix shutdown behavior
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15615 ) Change subject: [subprocess] Fix shutdown behavior .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15615/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java: http://gerrit.cloudera.org:8080/#/c/15615/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java@68 PS1, Line 68: System.exit(1); > If a RuntimeException is thrown and isn't caught, that thread will be stopp If we're calling System.exit(), we don't need to throw a new exception, right? That's effectively dead code. -- To view, visit http://gerrit.cloudera.org:8080/15615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia84bff891eaff667d7b22e89c66f24980ad76d70 Gerrit-Change-Number: 15615 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor 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: Tue, 31 Mar 2020 20:22:23 + Gerrit-HasComments: Yes
[kudu-CR] client: add support for columnar format scan
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15622 ) Change subject: client: add support for columnar format scan .. Patch Set 1: (3 comments) Just looked at the new APIs. http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/CMakeLists.txt File src/kudu/client/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/CMakeLists.txt@177 PS1, Line 177: # Headers: client Need to add the new header here. http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/client.h@2414 PS1, Line 2414: static const uint64_t COLUMNAR_LAYOUT = 1 << 1; Technically this flag is part of the "advanced/unstable" API (as per L2390). That what you want? http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/columnar_scan_batch.h File src/kudu/client/columnar_scan_batch.h: http://gerrit.cloudera.org:8080/#/c/15622/1/src/kudu/client/columnar_scan_batch.h@17 PS1, Line 17: #pragma once No good for pre-C++11. -- To view, visit http://gerrit.cloudera.org:8080/15622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69ec4487c78b872e7b9eba5facdd44cfd6b8f4fa Gerrit-Change-Number: 15622 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 31 Mar 2020 20:16:36 + Gerrit-HasComments: Yes
[kudu-CR] external mini cluster: actually default to system unsync time source
Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/14836 ) Change subject: external_mini_cluster: actually default to system_unsync time source .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/14836 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I1eefdea607d5631e11868a49341f51e120a0ba36 Gerrit-Change-Number: 14836 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP: use ExternalProject Add to define thirdparty deps
Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9931 ) Change subject: WIP: use ExternalProject_Add to define thirdparty deps .. Abandoned If we wanted to revisit our thirdparty dependency system, something like Bazel is probably the better choice. -- To view, visit http://gerrit.cloudera.org:8080/9931 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I4dec48958bf09f8f1a0d8af08c815a22d8240c3d Gerrit-Change-Number: 9931 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [thirdparty] root can run postgres
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15613 ) Change subject: [thirdparty] root can run postgres .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If6501648140bed3ba0df08a468cdf22a84f88cfc Gerrit-Change-Number: 15613 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 31 Mar 2020 06:24:59 + Gerrit-HasComments: No
[kudu-CR] [client] Add a note that only FAST HASH is supported in Bloom filter predicate
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15608 ) Change subject: [client] Add a note that only FAST_HASH is supported in Bloom filter predicate .. [client] Add a note that only FAST_HASH is supported in Bloom filter predicate Add explicit note that only FAST_HASH is supported in the Bloom filter predicate in client API. There is already validation in BlockBloomFilter for hash algorithm. Test: - Verified the generated HTML output. Change-Id: I5c683fe3be3e9ad2168562c5940e500fffac5316 Reviewed-on: http://gerrit.cloudera.org:8080/15608 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/client/scan_predicate.h 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5c683fe3be3e9ad2168562c5940e500fffac5316 Gerrit-Change-Number: 15608 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] Add a note that only FAST HASH is supported in Bloom filter predicate
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15608 ) Change subject: [client] Add a note that only FAST_HASH is supported in Bloom filter predicate .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5c683fe3be3e9ad2168562c5940e500fffac5316 Gerrit-Change-Number: 15608 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 31 Mar 2020 05:26:45 + Gerrit-HasComments: No
[kudu-CR] KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15605 ) Change subject: KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace .. KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace A TSAN build yielded a stack trace like: @ 0x444a88 __tsan::ProcessPendingSignals() @ 0x4541c1 __interceptor_pthread_mutex_trylock @ 0x7fbca26124e1 kudu::Mutex::TryAcquire() @ 0x7fbca2612893 kudu::Mutex::Acquire() @ 0x4fe036 kudu::MutexLock::MutexLock() @ 0x504abd kudu::CountDownLatch::WaitUntil() @ 0x504a5f kudu::CountDownLatch::WaitFor() @ 0x4f04a9 kudu::(anonymous namespace)::SleeperThread() ... Rather than find the synchronization primitive frame least likely to be inlined, let's take a more comprehensive approach and search for multiple candidate frames, including SleeperThread. I tested this locally in DEBUG, RELEASE, ASAN, and TSAN modes. Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 Reviewed-on: http://gerrit.cloudera.org:8080/15605 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/util/debug-util-test.cc 1 file changed, 12 insertions(+), 5 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 Gerrit-Change-Number: 15605 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] gutil: remove callback and bind from the codebase
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15583 ) Change subject: gutil: remove callback and bind from the codebase .. gutil: remove callback and bind from the codebase It is fitting that 5.5 years after bringing this code into Kudu (see commit 31f072096), I'm lucky enough to be the one burying it. There are no more usages of this functionality; everything has been migrated to std::function with lambdas for binding/capturing. Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Reviewed-on: http://gerrit.cloudera.org:8080/15583 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Alexey Serbin --- M build-support/release/rat_exclude_files.txt M docs/contributing.adoc M src/kudu/gutil/CMakeLists.txt D src/kudu/gutil/bind.h D src/kudu/gutil/bind.h.pump D src/kudu/gutil/bind_helpers.h D src/kudu/gutil/bind_internal.h D src/kudu/gutil/bind_internal.h.pump D src/kudu/gutil/callback.h D src/kudu/gutil/callback.h.pump D src/kudu/gutil/callback_forward.h D src/kudu/gutil/callback_internal.cc D src/kudu/gutil/callback_internal.h D src/kudu/gutil/raw_scoped_refptr_mismatch_checker.h M src/kudu/util/CMakeLists.txt D src/kudu/util/callback_bind-test.cc 16 files changed, 11 insertions(+), 6,046 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Gerrit-Change-Number: 15583 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2059: add a TSAN suppression
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15603 ) Change subject: KUDU-2059: add a TSAN suppression .. KUDU-2059: add a TSAN suppression No one is actively working on fixing this, so let's at least suppress it so that precommits aren't flaky. Change-Id: I86979e4c511bd4cbf027c629c867378cd0b8cd32 Reviewed-on: http://gerrit.cloudera.org:8080/15603 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/util/sanitizer_options.cc 1 file changed, 5 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15603 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I86979e4c511bd4cbf027c629c867378cd0b8cd32 Gerrit-Change-Number: 15603 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Remove return types from various lambdas
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15599 ) Change subject: Remove return types from various lambdas .. Remove return types from various lambdas Only the ones that were absolutely necessary (i.e. for implicit conversions) were retained. Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Reviewed-on: http://gerrit.cloudera.org:8080/15599 Tested-by: Adar Dembo Reviewed-by: Bankim Bhavsar Reviewed-by: Andrew Wong Reviewed-by: Alexey Serbin --- M src/kudu/client/client.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/hms/hms_client-test.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/sentry/sentry_client-test.cc M src/kudu/tablet/rowset_tree-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_perf.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/threadpool-test.cc M src/kudu/util/trace.h 27 files changed, 53 insertions(+), 54 deletions(-) Approvals: Adar Dembo: Verified Bankim Bhavsar: Looks good to me, but someone else must approve Andrew Wong: Looks good to me, approved Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar
[kudu-CR] [util] Add special handling for nullptr in fast hash
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15600 ) Change subject: [util] Add special handling for nullptr in fast hash .. [util] Add special handling for nullptr in fast hash Importing special handling for nullptr from Impala that helps distinguish between nullptr with possibly non-zero length input and empty object. https://github.com/apache/impala/blob/master/be/src/runtime/raw-value.inline.h#L40 https://github.com/apache/impala/blob/master/be/src/runtime/raw-value-ir.cc#L179 Fast hash already handles zero-length input and hence no special handling for zero length input. Fast hash is currently only used for BlockBloomFilter and hence no backward compatibility concerns. Don't plan to use Murmur2 hash with BlockBloomFilter and hence not importing the same logic for Murmur2 or other hash functions. Importing the same logic for other hash functions will require taking backward compatibility into account. Additionally this change updates Java implementation of Fast hash to drop the explicit "len" parameter as arrays in Java include length. Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Reviewed-on: http://gerrit.cloudera.org:8080/15600 Reviewed-by: Wenzhe Zhou Reviewed-by: Adar Dembo Tested-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java M java/kudu-client/src/test/java/org/apache/kudu/util/TestFashHash.java M src/kudu/util/hash_util-test.cc M src/kudu/util/hash_util.h 4 files changed, 79 insertions(+), 14 deletions(-) Approvals: Wenzhe Zhou: 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/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Wenzhe Zhou
[kudu-CR] [util] Add special handling for nullptr in fast hash
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15600 ) Change subject: [util] Add special handling for nullptr in fast hash .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Wenzhe Zhou
[kudu-CR] [util] Add special handling for nullptr in fast hash
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15600 ) Change subject: [util] Add special handling for nullptr in fast hash .. Patch Set 3: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 31 Mar 2020 05:23:46 + Gerrit-HasComments: No
[kudu-CR] WIP [release notes] support for RHEL/CentOS 8.1
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15594 ) Change subject: WIP [release notes] support for RHEL/CentOS 8.1 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia5986e4762516cfff5861980f404e78f953bf556 Gerrit-Change-Number: 15594 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 31 Mar 2020 05:20:17 + Gerrit-HasComments: No
[kudu-CR] Remove return types from various lambdas
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15599 )
Change subject: Remove return types from various lambdas
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15599/3/src/kudu/util/threadpool-test.cc
File src/kudu/util/threadpool-test.cc:
http://gerrit.cloudera.org:8080/#/c/15599/3/src/kudu/util/threadpool-test.cc@777
PS3, Line 777: auto GetRandomToken = [&]() {
> Nit: Need to look at datatype of "tokens" outside the lambda to determine t
I'm torn. On the one hand, I agree with you. On the other, lambdas (especially
in C++11) are already pretty verbose, and so I'm tempted to remove as much
unnecessary stuff as possible, especially in test code.
--
To view, visit http://gerrit.cloudera.org:8080/15599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9
Gerrit-Change-Number: 15599
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Comment-Date: Mon, 30 Mar 2020 22:53:52 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15605 to look at the new patch set (#2). Change subject: KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace .. KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace A TSAN build yielded a stack trace like: @ 0x444a88 __tsan::ProcessPendingSignals() @ 0x4541c1 __interceptor_pthread_mutex_trylock @ 0x7fbca26124e1 kudu::Mutex::TryAcquire() @ 0x7fbca2612893 kudu::Mutex::Acquire() @ 0x4fe036 kudu::MutexLock::MutexLock() @ 0x504abd kudu::CountDownLatch::WaitUntil() @ 0x504a5f kudu::CountDownLatch::WaitFor() @ 0x4f04a9 kudu::(anonymous namespace)::SleeperThread() ... Rather than find the synchronization primitive frame least likely to be inlined, let's take a more comprehensive approach and search for multiple candidate frames, including SleeperThread. I tested this locally in DEBUG, RELEASE, ASAN, and TSAN modes. Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 --- M src/kudu/util/debug-util-test.cc 1 file changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/15605/2 -- To view, visit http://gerrit.cloudera.org:8080/15605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 Gerrit-Change-Number: 15605 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15605 ) Change subject: KUDU-3093: another band-aid for this DebugUtilTest.TestSignalStackTrace .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15605/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15605/1//COMMIT_MSG@7 PS1, Line 7: this Debu > Could you explicitly mention the test name? Done -- To view, visit http://gerrit.cloudera.org:8080/15605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 Gerrit-Change-Number: 15605 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 22:52:37 + Gerrit-HasComments: Yes
[kudu-CR] [util] Add special handling for nullptr in fast hash
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15600 ) Change subject: [util] Add special handling for nullptr in fast hash .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java File java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java: http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java@24 PS1, Line 24: // Constant imported from Apache Impala used to compute hash values for special cases. : // Constant obtained by taking lower bytes of generated UUID. > > 1. What's the "generated UUID" you're alluding to? HASH_VAL_NULL is only 4 bytes though, and a UUID is typically 8 bytes. How was this generated? How is it guaranteed to be universally unique? This comment from Impala's raw-value.inline.h answers my question somewhat: /// Arbitrary constants used to compute hash values for special cases. Constants were /// obtained by taking lower bytes of generated UUID. NULL and empty strings should /// hash to different values. Could you work some of this information into this comment? Regarding the importance of the match, could you also work that into the comment? -- To view, visit http://gerrit.cloudera.org:8080/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 30 Mar 2020 22:51:43 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3093: another band-aid for this test
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15605 to review the following change. Change subject: KUDU-3093: another band-aid for this test .. KUDU-3093: another band-aid for this test A TSAN build yielded a stack trace like: @ 0x444a88 __tsan::ProcessPendingSignals() @ 0x4541c1 __interceptor_pthread_mutex_trylock @ 0x7fbca26124e1 kudu::Mutex::TryAcquire() @ 0x7fbca2612893 kudu::Mutex::Acquire() @ 0x4fe036 kudu::MutexLock::MutexLock() @ 0x504abd kudu::CountDownLatch::WaitUntil() @ 0x504a5f kudu::CountDownLatch::WaitFor() @ 0x4f04a9 kudu::(anonymous namespace)::SleeperThread() ... Rather than find the synchronization primitive frame least likely to be inlined, let's take a more comprehensive approach and search for multiple candidate frames, including SleeperThread. I tested this locally in DEBUG, RELEASE, ASAN, and TSAN modes. Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 --- M src/kudu/util/debug-util-test.cc 1 file changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/15605/1 -- To view, visit http://gerrit.cloudera.org:8080/15605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia4ca0f48ba1d7ad4cea40b70af271d7948f78a57 Gerrit-Change-Number: 15605 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] Remove return types from various lambdas
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15599 ) Change subject: Remove return types from various lambdas .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] Remove return types from various lambdas
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15599 ) Change subject: Remove return types from various lambdas .. Patch Set 3: Verified+1 Overriding Jenkins, unrelated test failure for which I'm generating a new patch. -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 22:12:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-2059: add a TSAN suppression
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15603 to review the following change. Change subject: KUDU-2059: add a TSAN suppression .. KUDU-2059: add a TSAN suppression No one is actively working on fixing this, so let's at least suppress it so that precommits aren't flaky. Change-Id: I86979e4c511bd4cbf027c629c867378cd0b8cd32 --- M src/kudu/util/sanitizer_options.cc 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/15603/1 -- To view, visit http://gerrit.cloudera.org:8080/15603 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I86979e4c511bd4cbf027c629c867378cd0b8cd32 Gerrit-Change-Number: 15603 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] Remove return types from various lambdas
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15599 ) Change subject: Remove return types from various lambdas .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] Remove return types from various lambdas
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15599 ) Change subject: Remove return types from various lambdas .. Patch Set 2: Verified+1 Overriding Jenkins, unrelated test failures (and KUDU-2059). -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 20:26:37 + Gerrit-HasComments: No
[kudu-CR] [util] Add special handling for nullptr in fast hash
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15600 ) Change subject: [util] Add special handling for nullptr in fast hash .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/15600/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15600/1//COMMIT_MSG@20 PS1, Line 20: compatibilty compatibility http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java File java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java: http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java@a92 PS1, Line 92: Perhaps the intent was to allow computing the hash on a slice of the array rather than the array in its entirety? It's mostly academic though: if we never actually used it that way, it doesn't matter. http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java@24 PS1, Line 24: // Constant imported from Apache Impala used to compute hash values for special cases. : // Constant obtained by taking lower bytes of generated UUID. Couple things aren't super clear here: 1. What's the "generated UUID" you're alluding to? 2. Is it important that this value matches the value in Apache Impala? If so, why? http://gerrit.cloudera.org:8080/#/c/15600/1/src/kudu/util/hash_util.h File src/kudu/util/hash_util.h: http://gerrit.cloudera.org:8080/#/c/15600/1/src/kudu/util/hash_util.h@32 PS1, Line 32: // Constant imported from Apache Impala used to compute hash values for special cases. If you end up changing the comment in the Java code to reflect my questions, please update this one too. -- To view, visit http://gerrit.cloudera.org:8080/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 20:24:36 + Gerrit-HasComments: Yes
[kudu-CR] gutil: remove callback and bind from the codebase
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15583 )
Change subject: gutil: remove callback and bind from the codebase
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/15583/4/docs/contributing.adoc
File docs/contributing.adoc:
http://gerrit.cloudera.org:8080/#/c/15583/4/docs/contributing.adoc@318
PS4, Line 318: Function Binding and Callbacks
> In other areas of this doc, we use "{cpp}11" with no space, which seems mor
Done
http://gerrit.cloudera.org:8080/#/c/15583/4/docs/contributing.adoc@320
PS4, Line 320: All code should use {cpp} 11 lambdas to capture and manage
functors. Functions that
: take a lambda as an argument should use `std::function` as the
argument's
: type. Do not use boost::bind or std::bind to create functors.
Lambdas offer the
: compiler greater opportunity to inline, and std::bind in
particular is
: link:https://abseil.io/tips/108[error-prone] and has a
proclivity towards heap
: allocation for storing bound parameters.
:
> nit: use backticks for the `bind` references?
Done
http://gerrit.cloudera.org:8080/#/c/15583/4/docs/contributing.adoc@327
PS4, Line 327: {cpp} 14
> As Andrew pointed, probably {cpp}14 would be the better way of writing it i
Done
http://gerrit.cloudera.org:8080/#/c/15583/4/docs/contributing.adoc@329
PS4, Line 329: {cpp} 11
> ditto
Done
--
To view, visit http://gerrit.cloudera.org:8080/15583
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9
Gerrit-Change-Number: 15583
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 30 Mar 2020 20:18:19 +
Gerrit-HasComments: Yes
[kudu-CR] gutil: remove callback and bind from the codebase
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15583 to look at the new patch set (#5). Change subject: gutil: remove callback and bind from the codebase .. gutil: remove callback and bind from the codebase It is fitting that 5.5 years after bringing this code into Kudu (see commit 31f072096), I'm lucky enough to be the one burying it. There are no more usages of this functionality; everything has been migrated to std::function with lambdas for binding/capturing. Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 --- M build-support/release/rat_exclude_files.txt M docs/contributing.adoc M src/kudu/gutil/CMakeLists.txt D src/kudu/gutil/bind.h D src/kudu/gutil/bind.h.pump D src/kudu/gutil/bind_helpers.h D src/kudu/gutil/bind_internal.h D src/kudu/gutil/bind_internal.h.pump D src/kudu/gutil/callback.h D src/kudu/gutil/callback.h.pump D src/kudu/gutil/callback_forward.h D src/kudu/gutil/callback_internal.cc D src/kudu/gutil/callback_internal.h D src/kudu/gutil/raw_scoped_refptr_mismatch_checker.h M src/kudu/util/CMakeLists.txt D src/kudu/util/callback_bind-test.cc 16 files changed, 11 insertions(+), 6,046 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/15583/5 -- To view, visit http://gerrit.cloudera.org:8080/15583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Gerrit-Change-Number: 15583 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] consensus: remove kudu::Bind usage from dirty callbacks
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15582 ) Change subject: consensus: remove kudu::Bind usage from dirty callbacks .. consensus: remove kudu::Bind usage from dirty callbacks And from various other miscellaneous places. Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Reviewed-on: http://gerrit.cloudera.org:8080/15582 Reviewed-by: Andrew Wong Reviewed-by: Alexey Serbin Tested-by: Adar Dembo --- M src/kudu/consensus/leader_election.cc M src/kudu/consensus/log.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/util/net/dns_resolver.cc M src/kudu/util/net/dns_resolver.h 16 files changed, 37 insertions(+), 61 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Alexey Serbin: Looks good to me, approved Adar Dembo: Verified -- To view, visit http://gerrit.cloudera.org:8080/15582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Gerrit-Change-Number: 15582 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] consensus: remove kudu::Bind usage from dirty callbacks
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15582 ) Change subject: consensus: remove kudu::Bind usage from dirty callbacks .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Gerrit-Change-Number: 15582 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] consensus: remove kudu::Bind usage from dirty callbacks
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15582 ) Change subject: consensus: remove kudu::Bind usage from dirty callbacks .. Patch Set 3: Verified+1 Overriding Jenkins, KUDU-2059. -- To view, visit http://gerrit.cloudera.org:8080/15582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Gerrit-Change-Number: 15582 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 20:16:11 + Gerrit-HasComments: No
[kudu-CR] Remove return types from various lambdas
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15599 to look at the new patch set (#2). Change subject: Remove return types from various lambdas .. Remove return types from various lambdas Only the ones that were absolutely necessary (i.e. for implicit conversions) were retained. Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 --- M src/kudu/client/client.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/hms/hms_client-test.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/sentry/sentry_client-test.cc M src/kudu/tablet/rowset_tree-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_perf.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/threadpool-test.cc M src/kudu/util/trace.h 27 files changed, 53 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/15599/2 -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] gutil: remove callback and bind from the codebase
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15583 to look at the new patch set (#4). Change subject: gutil: remove callback and bind from the codebase .. gutil: remove callback and bind from the codebase It is fitting that 5.5 years after bringing this code into Kudu (see commit 31f072096), I'm lucky enough to be the one burying it. There are no more usages of this functionality; everything has been migrated to std::function with lambdas for binding/capturing. Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 --- M build-support/release/rat_exclude_files.txt M docs/contributing.adoc M src/kudu/gutil/CMakeLists.txt D src/kudu/gutil/bind.h D src/kudu/gutil/bind.h.pump D src/kudu/gutil/bind_helpers.h D src/kudu/gutil/bind_internal.h D src/kudu/gutil/bind_internal.h.pump D src/kudu/gutil/callback.h D src/kudu/gutil/callback.h.pump D src/kudu/gutil/callback_forward.h D src/kudu/gutil/callback_internal.cc D src/kudu/gutil/callback_internal.h D src/kudu/gutil/raw_scoped_refptr_mismatch_checker.h M src/kudu/util/CMakeLists.txt D src/kudu/util/callback_bind-test.cc 16 files changed, 11 insertions(+), 6,046 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/15583/4 -- To view, visit http://gerrit.cloudera.org:8080/15583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Gerrit-Change-Number: 15583 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] gutil: remove BASE EXPORT
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15576 ) Change subject: gutil: remove BASE_EXPORT .. gutil: remove BASE_EXPORT The various definitions of BASE_EXPORT cause IWYU to recommend the inclusion of thread_collision_warner.h as a means of satisfying them. Since it serves no purpose in Kudu, let's remove the macro altogether. Change-Id: I8e1650f3a18b17898ab8082923fc2fb9c8471946 Reviewed-on: http://gerrit.cloudera.org:8080/15576 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/gutil/atomicops-internals-tsan.h M src/kudu/gutil/bind_helpers.h M src/kudu/gutil/ref_counted_memory.h M src/kudu/gutil/threading/thread_collision_warner.h M src/kudu/util/debug/trace_event.h M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/debug/trace_event_impl.h M src/kudu/util/debug/trace_event_synthetic_delay.h 8 files changed, 33 insertions(+), 54 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15576 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8e1650f3a18b17898ab8082923fc2fb9c8471946 Gerrit-Change-Number: 15576 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] consensus: remove kudu::Bind usage from dirty callbacks
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15582 ) Change subject: consensus: remove kudu::Bind usage from dirty callbacks .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/master/catalog_manager.cc@1305 PS2, Line 1305: -> Status > nit: I'm curious why is it necessary to specify the return type explicitly? Ah, I didn't realize it was optional. I've put together another patch to remove unnecessary return value types from lambdas across the codebase. http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/master/sys_catalog.cc File src/kudu/master/sys_catalog.cc: http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/master/sys_catalog.cc@394 PS2, Line 394: string > Or does lambda capture the ref? Maybe comment if so. Regardless, should at The lambda captures by value (i.e. copy), so Alexey is right that we needn't make yet another copy before declaring the lambda. http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tablet/tablet_replica-test.cc File src/kudu/tablet/tablet_replica-test.cc: http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tablet/tablet_replica-test.cc@145 PS2, Line 145: string tablet_id > nit: const auto& ? Done http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tserver/tablet_copy_source_session-test.cc File src/kudu/tserver/tablet_copy_source_session-test.cc: http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tserver/tablet_copy_source_session-test.cc@160 PS2, Line 160: string tablet_id > nit: const auto& ? Done http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/15582/2/src/kudu/tserver/ts_tablet_manager.cc@830 PS2, Line 830: const string& tablet_id > Make this consistent with your other tablet_id captures? Or vice versa. Yeah will change this to cauto& -- To view, visit http://gerrit.cloudera.org:8080/15582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Gerrit-Change-Number: 15582 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 18:55:36 + Gerrit-HasComments: Yes
[kudu-CR] Remove return types from various lambdas
Adar Dembo has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15599 Change subject: Remove return types from various lambdas .. Remove return types from various lambdas Only the ones that were absolutely necessary (i.e. for implicit conversions) were retained. Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 --- M docs/contributing.adoc M src/kudu/client/client.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/hms/hms_client-test.cc M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/sentry/sentry_client-test.cc M src/kudu/tablet/rowset_tree-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_perf.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/threadpool-test.cc M src/kudu/util/trace.h 28 files changed, 64 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/15599/1 -- To view, visit http://gerrit.cloudera.org:8080/15599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idb6cb2570121d47dee15d9e63b677126c30de4f9 Gerrit-Change-Number: 15599 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo
[kudu-CR] rpc: remove kudu::Bind usage from ServerPicker
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15578 ) Change subject: rpc: remove kudu::Bind usage from ServerPicker .. rpc: remove kudu::Bind usage from ServerPicker Snuck in a Bind removal from batcher.cc too. Change-Id: I40554f59152eec1b84171f16f1fa2b91aa3a1e55 Reviewed-on: http://gerrit.cloudera.org:8080/15578 Tested-by: Adar Dembo Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/client/batcher.cc M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc.h 6 files changed, 18 insertions(+), 20 deletions(-) Approvals: Adar Dembo: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I40554f59152eec1b84171f16f1fa2b91aa3a1e55 Gerrit-Change-Number: 15578 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] tracing: remove kudu::Bind usage from various callbacks
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15580 ) Change subject: tracing: remove kudu::Bind usage from various callbacks .. tracing: remove kudu::Bind usage from various callbacks Change-Id: I0ecad2771dd77b8532bffb983866898e26ef8aa3 Reviewed-on: http://gerrit.cloudera.org:8080/15580 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/debug/trace_event_impl.h 2 files changed, 24 insertions(+), 23 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0ecad2771dd77b8532bffb983866898e26ef8aa3 Gerrit-Change-Number: 15580 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] logging: remove kudu::Bind usage from LoggingCallback
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15577 ) Change subject: logging: remove kudu::Bind usage from LoggingCallback .. logging: remove kudu::Bind usage from LoggingCallback Change-Id: I8d3c995617ace2a5974be1104fa6f22b3ee5acea Reviewed-on: http://gerrit.cloudera.org:8080/15577 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/client/client.cc M src/kudu/util/logging.cc M src/kudu/util/logging_callback.h 3 files changed, 15 insertions(+), 17 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8d3c995617ace2a5974be1104fa6f22b3ee5acea Gerrit-Change-Number: 15577 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] fs: remove kudu::Bind usage from ErrorManager
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15579 ) Change subject: fs: remove kudu::Bind usage from ErrorManager .. fs: remove kudu::Bind usage from ErrorManager Change-Id: I15df4eda0e205535df5ec3700f66164840a1b183 Reviewed-on: http://gerrit.cloudera.org:8080/15579 Tested-by: Adar Dembo Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/fs/error_manager-test.cc M src/kudu/fs/error_manager.cc M src/kudu/fs/error_manager.h M src/kudu/fs/fs_manager.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/master/master.cc M src/kudu/tserver/tablet_server.cc 7 files changed, 48 insertions(+), 42 deletions(-) Approvals: Adar Dembo: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I15df4eda0e205535df5ec3700f66164840a1b183 Gerrit-Change-Number: 15579 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] consensus: remove kudu::Bind usage from dirty callbacks
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15582 to look at the new patch set (#3). Change subject: consensus: remove kudu::Bind usage from dirty callbacks .. consensus: remove kudu::Bind usage from dirty callbacks And from various other miscellaneous places. Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd --- M src/kudu/consensus/leader_election.cc M src/kudu/consensus/log.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/util/net/dns_resolver.cc M src/kudu/util/net/dns_resolver.h 16 files changed, 37 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15582/3 -- To view, visit http://gerrit.cloudera.org:8080/15582 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd Gerrit-Change-Number: 15582 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] env: remove kudu::Bind usage from Walk
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15575 ) Change subject: env: remove kudu::Bind usage from Walk .. env: remove kudu::Bind usage from Walk This isn't as elegant as the other patches in this series due to the number of parameters in WalkCallback. C++14 allows the use of 'auto' for parameter types which can shorten them somewhat. Change-Id: I080809aef7e2f0ac3c240d4ca6909951b9615bb0 Reviewed-on: http://gerrit.cloudera.org:8080/15575 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/fs/block_manager-test.cc M src/kudu/fs/file_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/env_util.cc M src/kudu/util/env_util.h M src/kudu/util/metrics.h 8 files changed, 50 insertions(+), 38 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I080809aef7e2f0ac3c240d4ca6909951b9615bb0 Gerrit-Change-Number: 15575 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] fs: remove kudu::Bind usage from DataDir closures
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15581 ) Change subject: fs: remove kudu::Bind usage from DataDir closures .. fs: remove kudu::Bind usage from DataDir closures Change-Id: Id58b8740ccc33762383a48680c726e8d30e7f25c Reviewed-on: http://gerrit.cloudera.org:8080/15581 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/fs/dir_manager.cc M src/kudu/fs/dir_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc 4 files changed, 33 insertions(+), 32 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id58b8740ccc33762383a48680c726e8d30e7f25c Gerrit-Change-Number: 15581 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] gutil: remove callback and bind from the codebase
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15583 )
Change subject: gutil: remove callback and bind from the codebase
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15583/2/docs/contributing.adoc
File docs/contributing.adoc:
http://gerrit.cloudera.org:8080/#/c/15583/2/docs/contributing.adoc@320
PS2, Line 320: All code should use C++11 lambdas to capture and manage
functors. Functions that
: take a lambda as an argument should use `std::function` as the
argument's type.
> +1
The ref-counting points are still valid. Having reviewed my changes, I bet you
saw things like:
std::function Foo::CreateLambda() {
scoped_refptr self(this);
return [self]() { self->DoSomething(); }
}
This code adds a ref to Foo, adds another ref to Foo when 'self' is copied into
the lambda, drops a ref from Foo when CreateLambda() returns, and drops another
ref from Foo later on, when the lambda goes out of scope and is destroyed.
In C++14: we'll be able to avoid the extra ref I think:
std::function Foo::CreateLambda() {
return [self = scoped_refptr(this)]() { self->DoSomething(); }
}
Anyway, kudu::Bind did that stuff automatically for us, but I don't think that
alone was a reason to keep using it.
--
To view, visit http://gerrit.cloudera.org:8080/15583
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9
Gerrit-Change-Number: 15583
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Comment-Date: Mon, 30 Mar 2020 18:55:33 +
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 ) Change subject: env: add a fifo class .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27 Gerrit-Change-Number: 15584 Gerrit-PatchSet: 5 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: Mon, 30 Mar 2020 18:17:12 + Gerrit-HasComments: No
[kudu-CR] WIP [release notes] support for RHEL/CentOS 8.1
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15594 ) Change subject: WIP [release notes] support for RHEL/CentOS 8.1 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15594/1/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/15594/1/docs/release_notes.adoc@50 PS1, Line 50: * Kudu now supports building and running on RHEL/CentOS 8.1. Do we explicitly _not_ support 8.0? Like, is Kudu unbuildable/unrunnable on 8.0? If not, maybe we should say something like how we support 8 (all flavors), but we ourselves have tested on 8.1? -- To view, visit http://gerrit.cloudera.org:8080/15594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia5986e4762516cfff5861980f404e78f953bf556 Gerrit-Change-Number: 15594 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 17:37:10 + Gerrit-HasComments: Yes
[kudu-CR] subprocess: use a fifo instead of stdout for IO
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15574 ) Change subject: subprocess: use a fifo instead of stdout for IO .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I63e91a090fe196713a013d28301c7980e452456c Gerrit-Change-Number: 15574 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong 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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 30 Mar 2020 17:34:35 + Gerrit-HasComments: No
[kudu-CR] env: add a fifo class
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return Status::IOError(Substitute("Error opening for $0:
$1", reason, filename));
> Done
Not done? I see you've changed NewFifo but not DoOpen here.
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 4
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: Mon, 30 Mar 2020 17:33:51 +
Gerrit-HasComments: Yes
[kudu-CR] subprocess: use a fifo instead of stdout for IO
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15574 )
Change subject: subprocess: use a fifo instead of stdout for IO
..
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15574/5/src/kudu/subprocess/subprocess_server-test.cc
File src/kudu/subprocess/subprocess_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/15574/5/src/kudu/subprocess/subprocess_server-test.cc@333
PS5, Line 333: for (int i = 0; i < kNumThreads; i++) {
: ASSERT_OK(results[i]);
: }
Can do a simpler for each loop on results.
--
To view, visit http://gerrit.cloudera.org:8080/15574
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e91a090fe196713a013d28301c7980e452456c
Gerrit-Change-Number: 15574
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 29 Mar 2020 23:15:47 +
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return Status::IOError(Substitute("Error opening for $0:
$1", reason, filename));
Can we return the results of the IOError() function here instead?
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 29 Mar 2020 23:12:40 +
Gerrit-HasComments: Yes
[kudu-CR] rpc: remove kudu::Bind usage from ServerPicker
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15578 ) Change subject: rpc: remove kudu::Bind usage from ServerPicker .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I40554f59152eec1b84171f16f1fa2b91aa3a1e55 Gerrit-Change-Number: 15578 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] rpc: remove kudu::Bind usage from ServerPicker
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15578 ) Change subject: rpc: remove kudu::Bind usage from ServerPicker .. Patch Set 2: Verified+1 Overriding Jenkins, known race. -- To view, visit http://gerrit.cloudera.org:8080/15578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40554f59152eec1b84171f16f1fa2b91aa3a1e55 Gerrit-Change-Number: 15578 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 29 Mar 2020 21:19:44 + Gerrit-HasComments: No
[kudu-CR] fs: remove kudu::Bind usage from ErrorManager
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15579 ) Change subject: fs: remove kudu::Bind usage from ErrorManager .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I15df4eda0e205535df5ec3700f66164840a1b183 Gerrit-Change-Number: 15579 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] fs: remove kudu::Bind usage from ErrorManager
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15579 ) Change subject: fs: remove kudu::Bind usage from ErrorManager .. Patch Set 2: Verified+1 Overriding Jenkins, known race. -- To view, visit http://gerrit.cloudera.org:8080/15579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I15df4eda0e205535df5ec3700f66164840a1b183 Gerrit-Change-Number: 15579 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 29 Mar 2020 21:18:25 + Gerrit-HasComments: No
[kudu-CR] gutil: remove callback and bind from the codebase
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15583 ) Change subject: gutil: remove callback and bind from the codebase .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Gerrit-Change-Number: 15583 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] gutil: remove callback and bind from the codebase
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15583 ) Change subject: gutil: remove callback and bind from the codebase .. Patch Set 2: Verified+1 Overriding Jenkins, known race. -- To view, visit http://gerrit.cloudera.org:8080/15583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5fb53730f9c33b37415b22fcc775266467116b9 Gerrit-Change-Number: 15583 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 29 Mar 2020 21:18:04 + Gerrit-HasComments: No
[kudu-CR] [build-support] introduce PARALLEL variable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15591 ) Change subject: [build-support] introduce PARALLEL variable .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1d5b36996d2b5e73f331a094c558abb97f895bc Gerrit-Change-Number: 15591 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 29 Mar 2020 21:01:01 + Gerrit-HasComments: No
[kudu-CR] [build-support] introduce PARALLEL variable
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15591 ) Change subject: [build-support] introduce PARALLEL variable .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15591/2/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: http://gerrit.cloudera.org:8080/#/c/15591/2/build-support/jenkins/build-and-test.sh@300 PS2, Line 300: $SOURCE_ROOT/build-support/enable_devtoolset.sh thirdparty/build-if-necessary.sh $THIRDPARTY_TYPE If you want you can add a comment saying that PARALLEL also affects the thirdparty build. -- To view, visit http://gerrit.cloudera.org:8080/15591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1d5b36996d2b5e73f331a094c558abb97f895bc Gerrit-Change-Number: 15591 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 29 Mar 2020 18:51:58 + Gerrit-HasComments: Yes
[kudu-CR] tracing: remove kudu::Bind usage from various callbacks
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15580 to look at the new patch set (#2). Change subject: tracing: remove kudu::Bind usage from various callbacks .. tracing: remove kudu::Bind usage from various callbacks Change-Id: I0ecad2771dd77b8532bffb983866898e26ef8aa3 --- M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/debug/trace_event_impl.h 2 files changed, 24 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/15580/2 -- To view, visit http://gerrit.cloudera.org:8080/15580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ecad2771dd77b8532bffb983866898e26ef8aa3 Gerrit-Change-Number: 15580 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
