[kudu-CR] KUDU-3011 p7: add tool to quiesce server
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15091 to look at the new patch set (#3). Change subject: KUDU-3011 p7: add tool to quiesce server .. KUDU-3011 p7: add tool to quiesce server Adds the following commands: $ kudu tserver quiesce - periodically sends a request to quiesce the server - the server will respond with the number of leaders and active scanners - once both of these are zero, exits with success - there are gflags to tune the timing of this $ kudu tserver stop_quiescing - sets the server to not be quiescing Tests: - Added some tests to exercise the quiescing tool in the context of a rolling restart alongside the maintenance mode tooling. - Also added some basic testing for the quiescing tooling alone. Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/maintenance_mode-itest.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_test_util.cc M src/kudu/tools/tool_test_util.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 12 files changed, 653 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/15091/3 -- To view, visit http://gerrit.cloudera.org:8080/15091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c Gerrit-Change-Number: 15091 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [clock] auto-config of built-in NTP client in cloud
Hello Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15070 to look at the new patch set (#2). Change subject: [clock] auto-config of built-in NTP client in cloud .. [clock] auto-config of built-in NTP client in cloud This patch introduces auto-configuration of the built-in NTP client in public cloud environment. Currently, AWS and GCE public cloud types are supported: Kudu masters and tablet servers are now capable of auto-detecting per-instance NTP server and using it as reference server for the built-in NTP client. The auto-configuration is controlled by the --builtin_ntp_client_enable_auto_config_in_cloud boolean flag and gated by the --time_source flag (i.e. the latter should be set to 'builtin' to allow the auto-configuration to work). Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb --- M src/kudu/clock/CMakeLists.txt M src/kudu/clock/builtin_ntp.cc M src/kudu/clock/builtin_ntp.h M src/kudu/clock/ntp-test.cc M src/kudu/mini-cluster/external_mini_cluster-test.cc 5 files changed, 121 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/15070/2 -- To view, visit http://gerrit.cloudera.org:8080/15070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb Gerrit-Change-Number: 15070 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3011 p7: add tool to quiesce server
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15091 to look at the new patch set (#2). Change subject: KUDU-3011 p7: add tool to quiesce server .. KUDU-3011 p7: add tool to quiesce server Adds the following commands: $ kudu tserver quiesce - periodically sends a request to quiesce the server - the server will respond with the number of leaders and active scanners - once both of these are zero, exits with success - there are gflags to tune the timing of this $ kudu tserver stop_quiescing - sets the server to not be quiescing Tests: - Added some tests to exercise the quiescing tool in the context of a rolling restart alongside the maintenance mode tooling. - Also added some basic testing for the quiescing tooling alone. Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/maintenance_mode-itest.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_test_util.cc M src/kudu/tools/tool_test_util.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 12 files changed, 652 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/15091/2 -- To view, visit http://gerrit.cloudera.org:8080/15091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c Gerrit-Change-Number: 15091 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3011 p7: add tool to quiesce server
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15091 Change subject: KUDU-3011 p7: add tool to quiesce server .. KUDU-3011 p7: add tool to quiesce server Adds the following commands: $ kudu tserver quiesce - periodically sends a request to quiesce the server - the server will respond with the number of leaders and active scanners - once both of these are zero, exits with success - there are gflags to tune the timing of this $ kudu tserver stop_quiescing - sets the server to not be quiescing Tests: - Added some tests to exercise the quiescing tool in the context of a rolling restart alongside the maintenance mode tooling. - Also added some basic testing for the quiescing tooling alone. Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/maintenance_mode-itest.cc M src/kudu/integration-tests/tablet_server_quiescing-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/tools/tool_test_util.cc M src/kudu/tools/tool_test_util.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 12 files changed, 649 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/15091/1 -- To view, visit http://gerrit.cloudera.org:8080/15091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I89657808cc2b0afc4e1b37ce75937ab12e098d9c Gerrit-Change-Number: 15091 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [utility] auto-detection of cloud VM instance type
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 ) Change subject: [utility] auto-detection of cloud VM instance type .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc File src/kudu/util/cloud/instance_detector-test.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc@83 PS4, Line 83: > To reduce possibility of flaky tests, can we go lower than this, FromNanose A few microsecond interval seems to be low enough because the interval covers spawning detector threads as well, and I ran it many 1K times in different configurations, not seeing any flakes. But I agree that MonoDelta::FromNanoseconds(1) is a better choice here. Done. http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h File src/kudu/util/cloud/instance_detector.h: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@38 PS4, Line 38: s run b > running-> run Done http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@56 PS4, Line 56: n didn't reco > auto-detection Done http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@60 PS4, Line 60: ut was > typo Done http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@70 PS4, Line 70: e specified inst > I can't find this field in this change. Perhaps it was renamed. Good catch: this has changed, indeed. I updated it. http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc File src/kudu/util/cloud/instance_detector.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc@84 PS4, Line 84: us wakeups are ignored by the > An instance can't be running on multiple cloud vendors, so the first one to Done -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Wed, 22 Jan 2020 01:43:42 + Gerrit-HasComments: Yes
[kudu-CR] [utility] auto-detection of cloud VM instance type
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Grant Henke, Greg Solovyev, Bankim Bhavsar, Volodymyr Verovkin, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14866 to look at the new patch set (#5). Change subject: [utility] auto-detection of cloud VM instance type .. [utility] auto-detection of cloud VM instance type Added a utility to auto-detect the type of VM instance for AWS, Azure and GCE public clouds. Follow-up changelists will make use of this functionality if auto-configuring the built-in NTP client upon startup of kudu-master and kudu-tserver. Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b --- M src/kudu/util/CMakeLists.txt A src/kudu/util/cloud/instance_detector-test.cc A src/kudu/util/cloud/instance_detector.cc A src/kudu/util/cloud/instance_detector.h A src/kudu/util/cloud/instance_metadata.cc A src/kudu/util/cloud/instance_metadata.h 6 files changed, 719 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14866/5 -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. Patch Set 4: Code-Review+2 For some reason, IWYU is still not happy: http://jenkins.kudu.apache.org/job/kudu-gerrit/20221/BUILD_TYPE=IWYU/artifact/build/latest/test-logs/iwyu.log -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 22 Jan 2020 01:42:16 + Gerrit-HasComments: No
[kudu-CR] schema: use dense hash map instead of std::unordered map
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15064 ) Change subject: schema: use dense_hash_map instead of std::unordered_map .. schema: use dense_hash_map instead of std::unordered_map In a time series benchmark I'm working on, the client spent 12% of its CPU in Schema::FindColumn. In particular, most of the CPU went to the bucket calculation in std::unordered_map, which required a 'divq' instruction that can take hundreds of cycles. This switches Schema to use a dense_hash_map instead which performs better. After this change, the percent of CPU used by my benchmark worker thread in Schema::FindColumn dropped from ~12% to ~1.5% which resulted in a few percent overall throughput increase. This also made the fancy allocator which tried to count memory usage unnecessary, since dense_hash_map is a simple enough data structure that we can directly compute the memory usage. Now we can also simplify the constructors since we no longer need to pass an allocator instance. Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Reviewed-on: http://gerrit.cloudera.org:8080/15064 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/client/client-test.cc M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/common/wire_protocol.cc M thirdparty/download-thirdparty.sh A thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch 6 files changed, 88 insertions(+), 86 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] file cache: support alternate open modes
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15081 ) Change subject: file cache: support alternate open modes .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/fs/log_block_manager.cc@805 PS1, Line 805: } while (PREDICT_FALSE(metadata_status.IsAlreadyPresent() || : data_status.IsAlreadyPresent())); Is it possible this cycle run infinitely if the file got some sort of fs-level error or has chflags/etc. set? http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.cc File src/kudu/util/file_cache.cc: http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.cc@557 PS1, Line 557: DCHECK(!FindDescriptorUnlocked(file_name, FindMode::DONT_CREATE, :&rwf_descs_, &ignored)); nit: wrap into #ifndef NDEBUG ... #endif as for the OpenFile(..., shared_ptr) method above? -- To view, visit http://gerrit.cloudera.org:8080/15081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie167302ef85b8e1a40fbb89a7742e2cbb43bcec3 Gerrit-Change-Number: 15081 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 23:35:22 + Gerrit-HasComments: Yes
[kudu-CR] schema: use dense hash map instead of std::unordered map
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15064 ) Change subject: schema: use dense_hash_map instead of std::unordered_map .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 23:13:09 + Gerrit-HasComments: No
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15042 to look at the new patch set (#4). Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 107 insertions(+), 100 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/15042/4 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] build: restrict clang version, prefer lld, enable thinlto
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15058 ) Change subject: build: restrict clang version, prefer lld, enable thinlto .. build: restrict clang version, prefer lld, enable thinlto * Restricts clang to at least version 6.0, which gets rid of a couple older special cases. * We now loop through linkers trying to prefer lld where available, either using the version provided by the compiler with -fuse-lld or explicitly passing the path to the toolchain lld. lld should be faster than gold and GNU ld, and also has the advantage of being a known quantity within our toolchain. This fixes ASAN builds on my Ubuntu 18 machine, where the version of gold that ships with the system can't seem to link ASAN executables. * Switches the LTO build to use ThinLTO, which provides most of the performance benefits but at much lower cost. Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Reviewed-on: http://gerrit.cloudera.org:8080/15058 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M CMakeLists.txt A cmake_modules/KuduLinker.cmake 2 files changed, 191 insertions(+), 120 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Gerrit-Change-Number: 15058 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] schema: use dense hash map instead of std::unordered map
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15064 to look at the new patch set (#5). Change subject: schema: use dense_hash_map instead of std::unordered_map .. schema: use dense_hash_map instead of std::unordered_map In a time series benchmark I'm working on, the client spent 12% of its CPU in Schema::FindColumn. In particular, most of the CPU went to the bucket calculation in std::unordered_map, which required a 'divq' instruction that can take hundreds of cycles. This switches Schema to use a dense_hash_map instead which performs better. After this change, the percent of CPU used by my benchmark worker thread in Schema::FindColumn dropped from ~12% to ~1.5% which resulted in a few percent overall throughput increase. This also made the fancy allocator which tried to count memory usage unnecessary, since dense_hash_map is a simple enough data structure that we can directly compute the memory usage. Now we can also simplify the constructors since we no longer need to pass an allocator instance. Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 --- M src/kudu/client/client-test.cc M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/common/wire_protocol.cc M thirdparty/download-thirdparty.sh A thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch 6 files changed, 88 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/15064/5 -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] schema: use dense hash map instead of std::unordered map
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15064 ) Change subject: schema: use dense_hash_map instead of std::unordered_map .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/15064/4/thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch File thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch: PS4: > Did you also submit this patch upstream? nope, last time I tried to submit a gcc 4.8 compatibility patch upstream they didn't seem keen on workarounds for gcc 4.8 (https://github.com/sparsehash/sparsehash-c11/pull/19) -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 22:50:11 + Gerrit-HasComments: Yes
[kudu-CR] file cache: evict open fd when descriptor goes out of scope
Adar Dembo has removed a vote on this change. Change subject: file cache: evict open fd when descriptor goes out of scope .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Gerrit-Change-Number: 15080 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] file cache: evict open fd when descriptor goes out of scope
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15080 ) Change subject: file cache: evict open fd when descriptor goes out of scope .. file cache: evict open fd when descriptor goes out of scope Previously, an open fd remained in cache when its descriptor was destroyed unless the file was marked for deletion. This avoided an extra cache lookup at destruction time and provided faster access to a file if it was closed and then reopened (though that second benefit was irrelevant to the block managers where files were kept open permanently unless they were deleted). With this change, an open fd is forcefully evicted when its descriptor is destroyed. This provides better semantics if the goal is to close a file without deleting it and test that the fd is indeed closed. It also prevents "false positive" cache hits when the user closes the file, renames it, and creates a new file with the old file name. This is an access pattern used by the WAL when a replica fails and it is brought back to life via tablet copy from a healthy server. Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Reviewed-on: http://gerrit.cloudera.org:8080/15080 Reviewed-by: Andrew Wong Tested-by: Adar Dembo --- M src/kudu/util/file_cache-test.cc M src/kudu/util/file_cache.cc M src/kudu/util/file_cache.h 3 files changed, 24 insertions(+), 16 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Adar Dembo: Verified -- To view, visit http://gerrit.cloudera.org:8080/15080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Gerrit-Change-Number: 15080 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] file cache: evict open fd when descriptor goes out of scope
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15080 ) Change subject: file cache: evict open fd when descriptor goes out of scope .. Patch Set 1: Verified+1 Overriding Jenkins, unrelated test failure. -- To view, visit http://gerrit.cloudera.org:8080/15080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Gerrit-Change-Number: 15080 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-Comment-Date: Tue, 21 Jan 2020 22:40:43 + Gerrit-HasComments: No
[kudu-CR] rpc-test-base.h: squelch a warning
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15079 ) Change subject: rpc-test-base.h: squelch a warning .. rpc-test-base.h: squelch a warning Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 Reviewed-on: http://gerrit.cloudera.org:8080/15079 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/rpc/rpc-test-base.h 1 file changed, 3 insertions(+), 1 deletion(-) 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/15079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 Gerrit-Change-Number: 15079 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] compression: fix handling of NO COMPRESSION
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15078 ) Change subject: compression: fix handling of NO_COMPRESSION .. compression: fix handling of NO_COMPRESSION The string values of CompressionType and GetCompressionCodecType() did not agree: the former used NO_COMPRESSION and the latter NONE to indicate the lack of compression. This led to some unnecessary warnings when a stringified CompressionType was fed into GetCompressionCodecType(), as is done in log-test. This patch changes GetCompressionCodecType() to expect NO_COMPRESSION rather than NONE. It shouldn't affect backwards compatibility: if someone really does use NONE (i.e. in a gflag), they'll just get no compression anyway, albeit with the ugly warning. That's not ideal, but the alternative (use NONE in CompressionType) may break backwards compatibility in JSON encoding, and NO_COMPRESSION is the value we use in our public APIs. Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 Reviewed-on: http://gerrit.cloudera.org:8080/15078 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Alexey Serbin --- M src/kudu/cfile/cfile_writer.cc M src/kudu/consensus/log-test.cc M src/kudu/consensus/log.cc M src/kudu/integration-tests/disk_reservation-itest.cc M src/kudu/integration-tests/raft_consensus-itest-base.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/util/compression/compression_codec.cc 8 files changed, 13 insertions(+), 23 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/15078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 Gerrit-Change-Number: 15078 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] tablet copy client: delete WAL data using existing function
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15077 ) Change subject: tablet copy client: delete WAL data using existing function .. tablet copy client: delete WAL data using existing function I thought this might be needed for correctness (if Log::DeleteOnDiskData did something beyond a recursive deletion), but it's not. Nevertheless, it seems more robust, if DeleteOnDiskData _did_ were changed to do something interesting in the future. Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d Reviewed-on: http://gerrit.cloudera.org:8080/15077 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 2 files changed, 9 insertions(+), 8 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d Gerrit-Change-Number: 15077 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] build: restrict clang version, prefer lld, enable thinlto
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15058 ) Change subject: build: restrict clang version, prefer lld, enable thinlto .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15058/4/cmake_modules/KuduLinker.cmake File cmake_modules/KuduLinker.cmake: http://gerrit.cloudera.org:8080/#/c/15058/4/cmake_modules/KuduLinker.cmake@68 PS4, Line 68: # [1] https://reviews.llvm.org/D63974 Might prefer to link to the actual bug itself rather than the CR: https://bugs.llvm.org/show_bug.cgi?id=42442. -- To view, visit http://gerrit.cloudera.org:8080/15058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Gerrit-Change-Number: 15058 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 22:37:23 + Gerrit-HasComments: Yes
[kudu-CR] log: start using file cache
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15082 ) Change subject: log: start using file cache .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15082/1/src/kudu/consensus/log.cc File src/kudu/consensus/log.cc: http://gerrit.cloudera.org:8080/#/c/15082/1/src/kudu/consensus/log.cc@1071 PS1, Line 1071: // Note: the segment files will only be deleted from disk when : // segments_to_delete goes out of scope. nit: Is this important? Could we iterate by mutable ref and reset the references in segments_to_delete? http://gerrit.cloudera.org:8080/#/c/15082/1/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/15082/1/src/kudu/tserver/ts_tablet_manager.cc@1127 PS1, Line 1127: server_->file_cache(), Using the file cache means there's a performance hit every time we allocate/roll onto new WAL segments, and it adds stress on data block pathways as well. Do you think we ought to add a flag that enables this? -- To view, visit http://gerrit.cloudera.org:8080/15082 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c00f6b839a693e059fa2ce79abf347dbf83bdd0 Gerrit-Change-Number: 15082 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 22:37:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 8: Verified+1 Code-Review+2 Looks good to me, though not merging yet since I think you wanted to collect additional reviews? -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 21 Jan 2020 22:33:04 + Gerrit-HasComments: No
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu
[kudu-CR] [utility] auto-detection of cloud VM instance type
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 ) Change subject: [utility] auto-detection of cloud VM instance type .. Patch Set 4: Code-Review+1 (6 comments) http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc File src/kudu/util/cloud/instance_detector-test.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector-test.cc@83 PS4, Line 83: FromMicroseconds To reduce possibility of flaky tests, can we go lower than this, FromNanoseconds()? http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h File src/kudu/util/cloud/instance_detector.h: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@38 PS4, Line 38: running running-> run http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@56 PS4, Line 56: autodetection auto-detection http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@60 PS4, Line 60: cloude typo http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.h@70 PS4, Line 70: result_metadata_ I can't find this field in this change. Perhaps it was renamed. http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc File src/kudu/util/cloud/instance_detector.cc: http://gerrit.cloudera.org:8080/#/c/14866/4/src/kudu/util/cloud/instance_detector.cc@84 PS4, Line 84: result_detector_idx_ == kNoIdx An instance can't be running on multiple cloud vendors, so the first one to update result_detector_idx_ wins the race and breaks the loop in case of spurious wake ups for other threads? Might be worth adding a comment. -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 21 Jan 2020 22:31:17 + Gerrit-HasComments: Yes
[kudu-CR] schema: use dense hash map instead of std::unordered map
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15064 ) Change subject: schema: use dense_hash_map instead of std::unordered_map .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15064/4/thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch File thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch: PS4: Did you also submit this patch upstream? -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 22:30:57 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 8: > Patch Set 8: Verified-1 > > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/20217/ : FAILURE Bad status: Timed out: ListTabletServers RPC failed: ListTabletServers RPC to 127.1.100.62:38687 timed out after 0.430s (SENT) Unrelated test failure: http://jenkins.kudu.apache.org/job/kudu-gerrit/20217/BUILD_TYPE=RELEASE/testReport/junit/(root)/TsLocationAssignmentITest/Basic_3/ -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 21 Jan 2020 22:20:09 + Gerrit-HasComments: No
[kudu-CR] schema: use dense hash map instead of std::unordered map
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15064 ) Change subject: schema: use dense_hash_map instead of std::unordered_map .. Patch Set 3: Hit a gcc 4.8 compatibility bug. Added a workaround, hopefully will compile (I couldn't get gcc 4.8 to work on my ubuntu 18 box) -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 21:58:24 + Gerrit-HasComments: No
[kudu-CR] schema: use dense hash map instead of std::unordered map
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15064 to look at the new patch set (#4). Change subject: schema: use dense_hash_map instead of std::unordered_map .. schema: use dense_hash_map instead of std::unordered_map In a time series benchmark I'm working on, the client spent 12% of its CPU in Schema::FindColumn. In particular, most of the CPU went to the bucket calculation in std::unordered_map, which required a 'divq' instruction that can take hundreds of cycles. This switches Schema to use a dense_hash_map instead which performs better. After this change, the percent of CPU used by my benchmark worker thread in Schema::FindColumn dropped from ~12% to ~1.5% which resulted in a few percent overall throughput increase. This also made the fancy allocator which tried to count memory usage unnecessary, since dense_hash_map is a simple enough data structure that we can directly compute the memory usage. Now we can also simplify the constructors since we no longer need to pass an allocator instance. Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 --- M src/kudu/client/client-test.cc M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/common/wire_protocol.cc M thirdparty/download-thirdparty.sh A thirdparty/patches/sparsehash-0002-Add-workaround-for-dense_hashtable-move-constructor-.patch 6 files changed, 87 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/15064/4 -- To view, visit http://gerrit.cloudera.org:8080/15064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8e8f80229b2dcfad05e204a6f6e50ce7dc3f4c73 Gerrit-Change-Number: 15064 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [spark] Replace bad Guava import
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15084 ) Change subject: [spark] Replace bad Guava import .. [spark] Replace bad Guava import TestImportExportFiles uses the shaded Spark import of ImmutableList. Instead we should use the actual Guava import. Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 Reviewed-on: http://gerrit.cloudera.org:8080/15084 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Bankim Bhavsar --- M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved Bankim Bhavsar: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/15084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 Gerrit-Change-Number: 15084 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] file cache: support alternate open modes
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15081 ) Change subject: file cache: support alternate open modes .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.h File src/kudu/util/file_cache.h: http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.h@125 PS1, Line 125: Every mode tries to behave as a POSIX filesystem would, but the : // semantics aren't 100% right when using the more "interesting" modes (such : // as CREATE_OR_OPEN_WITH_TRUNCATE) and deleting files concurrently. Beware. It it worth enumerating examples of how it differs? Also if it's not used at all, would you be against punting on supporting that mode entirely? Especially if it's not used anywhere in our codebase. Eg by DCHECKing unsupported inputs. http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.cc File src/kudu/util/file_cache.cc: http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.cc@340 PS1, Line 340: out nit: maybe rename the variable to 'existing_file' then so that's a bit more obvious? http://gerrit.cloudera.org:8080/#/c/15081/1/src/kudu/util/file_cache.cc@360 PS1, Line 360: // TODO(adar): this doesn't need to be a member as it's only used by Init, but : // there's no easy way to plumb it through KuduOnceDynamic. I wonder if it would help if we were to templatize ReopenFileIfNecessary with the OpenMode? And then call it with in InitOnce(). Though punting is fine too. -- To view, visit http://gerrit.cloudera.org:8080/15081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie167302ef85b8e1a40fbb89a7742e2cbb43bcec3 Gerrit-Change-Number: 15081 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 21:42:14 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Replace bad Guava import
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15084 ) Change subject: [spark] Replace bad Guava import .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 Gerrit-Change-Number: 15084 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 21:43:09 + Gerrit-HasComments: No
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Hello Kudu Jenkins, helifu, Yao Xu, Adar Dembo, ZhangYao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15034 to look at the new patch set (#8). Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side This change switches the implementation of the ColumnPredicate to use the BlockBloomFilter for the BloomFilter predicate on the server side. Earlier implementation was still experimental and didn't provide public client APIs that actually use this BloomFilter predicate so taken the liberty to make incompatible wire protocol changes. Updated BlockBloomFilter to take hash_algorithm and hash_seed. This make serializing and deserializing the BlockBloomFilter convenient and removes the need of BloomFilterInner in ColumnPredicate. Added overloaded Insert()/Find() functions to BlockBloomFilter that take Slice parameter and hashes the key before insertion/lookup. Most of the change involves refactoring the implementation including the unit tests. Currently only FAST_HASH algorithm is supported since 32-bit versions of MURMUR2 and CITY_HASH are not currently implemented. Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a --- M src/kudu/common/column_predicate-test.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h M src/kudu/common/common.proto M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc M src/kudu/tablet/cfile_set-test.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/block_bloom_filter-test.cc M src/kudu/util/block_bloom_filter.cc M src/kudu/util/block_bloom_filter.h A src/kudu/util/block_bloom_filter.proto M src/kudu/util/hash.proto M src/kudu/util/hash_util-test.cc M src/kudu/util/hash_util.h 15 files changed, 514 insertions(+), 390 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/15034/8 -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 8 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
Bankim Bhavsar has posted comments on this change. (
http://gerrit.cloudera.org:8080/15034 )
Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on
server side
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15034/7/src/kudu/util/block_bloom_filter.cc
File src/kudu/util/block_bloom_filter.cc:
http://gerrit.cloudera.org:8080/#/c/15034/7/src/kudu/util/block_bloom_filter.cc@74
PS7, Line 74: DCHECK(directory_ == nullptr) <<
: "Close() should have been called before the object is
destroyed.";
> If the destructor is going to call Close(), doesn't seem like we need this
Done
http://gerrit.cloudera.org:8080/#/c/15034/7/src/kudu/util/block_bloom_filter.cc@220
PS7, Line 220: void BlockBloomFilter::Insert(const Slice& key) noexcept {
: Insert(HashUtil::ComputeHash32(key, hash_algorithm_,
hash_seed_));
: }
> This and Find() could be inlined in the header.
Done
--
To view, visit http://gerrit.cloudera.org:8080/15034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a
Gerrit-Change-Number: 15034
Gerrit-PatchSet: 7
Gerrit-Owner: Bankim Bhavsar
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: ZhangYao
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 21 Jan 2020 21:42:08 +
Gerrit-HasComments: Yes
[kudu-CR] build: restrict clang version, prefer lld, enable thinlto
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15058 to look at the new patch set (#4). Change subject: build: restrict clang version, prefer lld, enable thinlto .. build: restrict clang version, prefer lld, enable thinlto * Restricts clang to at least version 6.0, which gets rid of a couple older special cases. * We now loop through linkers trying to prefer lld where available, either using the version provided by the compiler with -fuse-lld or explicitly passing the path to the toolchain lld. lld should be faster than gold and GNU ld, and also has the advantage of being a known quantity within our toolchain. This fixes ASAN builds on my Ubuntu 18 machine, where the version of gold that ships with the system can't seem to link ASAN executables. * Switches the LTO build to use ThinLTO, which provides most of the performance benefits but at much lower cost. Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 --- M CMakeLists.txt A cmake_modules/KuduLinker.cmake 2 files changed, 191 insertions(+), 120 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/15058/4 -- To view, visit http://gerrit.cloudera.org:8080/15058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Gerrit-Change-Number: 15058 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] build: restrict clang version, prefer lld, enable thinlto
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15058 ) Change subject: build: restrict clang version, prefer lld, enable thinlto .. Patch Set 3: (3 comments) turns out there's a bug in LLD with weak symbol handling. Added a workaround. http://gerrit.cloudera.org:8080/#/c/15058/3/cmake_modules/KuduLinker.cmake File cmake_modules/KuduLinker.cmake: http://gerrit.cloudera.org:8080/#/c/15058/3/cmake_modules/KuduLinker.cmake@38 PS3, Line 38: # How we handle this situation depends on other factors: : # - If gold is optional, we won't use it. : # - If gold is required and we're using dynamic linking, we'll either: : # - Raise an error in RELEASE builds (we shouldn't release such a product), or : # - Drop tcmalloc in all other builds. > Forgot to mention earlier, but this needs to be updated to reflect the new Done http://gerrit.cloudera.org:8080/#/c/15058/3/cmake_modules/KuduLinker.cmake@61 PS3, Line 61: > Whitespace. Done http://gerrit.cloudera.org:8080/#/c/15058/3/cmake_modules/KuduLinker.cmake@112 PS3, Line 112: # GNU ld version 2.25.1-22.base.el7 > Trailing whitespace. Done -- To view, visit http://gerrit.cloudera.org:8080/15058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09ded0c44c9f7a6839489f0abf5baa4eaf1971f0 Gerrit-Change-Number: 15058 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Jan 2020 21:15:32 + Gerrit-HasComments: Yes
[kudu-CR] [clock] auto-config of built-in NTP client in cloud
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15070 ) Change subject: [clock] auto-config of built-in NTP client in cloud .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc File src/kudu/clock/builtin_ntp.cc: http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc@593 PS1, Line 593: RETURN_NOT_OK_PREPEND(HostPort::ParseStrings(FLAGS_builtin_ntp_servers, : kStandardNtpPort, &hps), : "could not parse --builtin_ntp_servers flag"); > Do you think we should also use these servers in addition to the cloud-prov I'm not sure these are needed by default if using the NTP server provided by the cloud infrastructure itself, because these * might be blocked by firewall * might be too far in terms of RTT and thus wouldn't constitute a set of good-enough candidates for time synchronization Maybe, we should add a flag to add those if really needed? Not sure what might be a use-case that justifies for adding those in addition to the NTP server provided from within a cloud instance. -- To view, visit http://gerrit.cloudera.org:8080/15070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb Gerrit-Change-Number: 15070 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 20:42:16 + Gerrit-HasComments: Yes
[kudu-CR] [utility] auto-detection of cloud VM instance type
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 ) Change subject: [utility] auto-detection of cloud VM instance type .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc File src/kudu/util/cloud/instance_metadata.cc: http://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.cc@86 PS1, Line 86: "169.254.169.123" > Thank you for the feedback. It seems some extra clarification is needed he All right, I added gflags to be able to control these. -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 21 Jan 2020 20:35:22 + Gerrit-HasComments: Yes
[kudu-CR] [utility] auto-detection of cloud VM instance type
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Grant Henke, Greg Solovyev, Bankim Bhavsar, Volodymyr Verovkin, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14866 to look at the new patch set (#4). Change subject: [utility] auto-detection of cloud VM instance type .. [utility] auto-detection of cloud VM instance type Added a utility to auto-detect the type of VM instance for AWS, Azure and GCE public clouds. Follow-up changelists will make use of this functionality if auto-configuring the built-in NTP client upon startup of kudu-master and kudu-tserver. Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b --- M src/kudu/util/CMakeLists.txt A src/kudu/util/cloud/instance_detector-test.cc A src/kudu/util/cloud/instance_detector.cc A src/kudu/util/cloud/instance_detector.h A src/kudu/util/cloud/instance_metadata.cc A src/kudu/util/cloud/instance_metadata.h 6 files changed, 708 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14866/4 -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin
[kudu-CR] [utility] auto-detection of cloud VM instance type
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14866 )
Change subject: [utility] auto-detection of cloud VM instance type
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14866/3/src/kudu/util/cloud/instance_detector.h
File src/kudu/util/cloud/instance_detector.h:
http://gerrit.cloudera.org:8080/#/c/14866/3/src/kudu/util/cloud/instance_detector.h@79
PS3, Line 79: struct DetectorInfo {
: std::unique_ptr metadata;
> Could combine into one vector with a struct.
Done
http://gerrit.cloudera.org:8080/#/c/14866/3/src/kudu/util/cloud/instance_detector.cc
File src/kudu/util/cloud/instance_detector.cc:
http://gerrit.cloudera.org:8080/#/c/14866/3/src/kudu/util/cloud/instance_detector.cc@62
PS3, Line 62: const auto deadline = MonoTime::Now() + timeout_;
> I think you could use a loop here, perhaps with a vectorhttp://gerrit.cloudera.org:8080/#/c/14866/1/src/kudu/util/cloud/instance_metadata.h
File src/kudu/util/cloud/instance_metadata.h:
PS1:
> That's true; I'm mostly worried about tests that do a fair amount of daemon
With a follow-up changelist, this feature is used by the external-mini-cluster
test and by the external_mini_cluster-test-test, so not many tests use it now.
--
To view, visit http://gerrit.cloudera.org:8080/14866
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b
Gerrit-Change-Number: 14866
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Bankim Bhavsar
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Greg Solovyev
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Volodymyr Verovkin
Gerrit-Comment-Date: Tue, 21 Jan 2020 20:34:00 +
Gerrit-HasComments: Yes
[kudu-CR] [spark] Replace bad Guava import
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15084 ) Change subject: [spark] Replace bad Guava import .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 Gerrit-Change-Number: 15084 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 20:18:13 + Gerrit-HasComments: No
[kudu-CR] file cache: evict open fd when descriptor goes out of scope
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15080 ) Change subject: file cache: evict open fd when descriptor goes out of scope .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Gerrit-Change-Number: 15080 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 20:15:32 + Gerrit-HasComments: No
[kudu-CR] rpc-test-base.h: squelch a warning
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15079 ) Change subject: rpc-test-base.h: squelch a warning .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 Gerrit-Change-Number: 15079 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 20:07:35 + Gerrit-HasComments: No
[kudu-CR] rpc-test-base.h: squelch a warning
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15079 ) Change subject: rpc-test-base.h: squelch a warning .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 Gerrit-Change-Number: 15079 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 19:36:51 + Gerrit-HasComments: No
[kudu-CR] compression: fix handling of NO COMPRESSION
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15078 ) Change subject: compression: fix handling of NO_COMPRESSION .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 Gerrit-Change-Number: 15078 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 19:36:28 + Gerrit-HasComments: No
[kudu-CR] tablet copy client: delete WAL data using existing function
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15077 ) Change subject: tablet copy client: delete WAL data using existing function .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d Gerrit-Change-Number: 15077 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 19:18:46 + Gerrit-HasComments: No
[kudu-CR] compression: fix handling of NO COMPRESSION
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15078 ) Change subject: compression: fix handling of NO_COMPRESSION .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 Gerrit-Change-Number: 15078 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 19:03:49 + Gerrit-HasComments: No
[kudu-CR] [spark] Replace bad Guava import
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15084 Change subject: [spark] Replace bad Guava import .. [spark] Replace bad Guava import TestImportExportFiles uses the shaded Spark import of ImmutableList. Instead we should use the actual Guava import. Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 --- M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15084/1 -- To view, visit http://gerrit.cloudera.org:8080/15084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id75b16ad80800df554cfd129a70c03d9250e2292 Gerrit-Change-Number: 15084 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [python] Use positional formatting in Python scripts
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15075 ) Change subject: [python] Use positional formatting in Python scripts .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Gerrit-Change-Number: 15075 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 18:44:21 + Gerrit-HasComments: No
[kudu-CR] [python] Use positional formatting in Python scripts
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15075 ) Change subject: [python] Use positional formatting in Python scripts .. [python] Use positional formatting in Python scripts get-job-stats-from-mysql.py is not compatible with Python 2.6.x beause it uses the format function without the positions included. I also updated other calls to format without explicit positions. Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Reviewed-on: http://gerrit.cloudera.org:8080/15075 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong Reviewed-by: Bankim Bhavsar --- M build-support/iwyu/iwyu_tool.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/graph-metrics.py M src/kudu/scripts/max_skew_estimate.py 5 files changed, 8 insertions(+), 8 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved Bankim Bhavsar: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/15075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Gerrit-Change-Number: 15075 Gerrit-PatchSet: 3 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)
[kudu-CR] [python] Use positional formatting in Python scripts
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15075 ) Change subject: [python] Use positional formatting in Python scripts .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/15075/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15075/2//COMMIT_MSG@10 PS2, Line 10: beause typo -- To view, visit http://gerrit.cloudera.org:8080/15075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Gerrit-Change-Number: 15075 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: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 18:44:28 + Gerrit-HasComments: Yes
[kudu-CR] tablet copy client: delete WAL data using existing function
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15077 ) Change subject: tablet copy client: delete WAL data using existing function .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d Gerrit-Change-Number: 15077 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 18:28:41 + Gerrit-HasComments: No
[kudu-CR] log: start using file cache
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15082 to review the following change. Change subject: log: start using file cache .. log: start using file cache This patch modifies the log to use the file cache. New WAL segments are opened through the cache from the moment we switch to them, meaning there's a short period of time as they're being preallocated where they're opened outside the cache. Log index chunks are only used through the cache. The bulk of the patch is plumbing to get the file cache down from the server into the various log classes. Most "interesting" log-related tests have been modified to instantiate a cache while other unit tests have not, ensuring a mix of test coverage. One important semantic change to be aware of: it is now unsafe to delete a log's data or bootstrap a new copy of an existing tablet without first closing the old log. Thankfully we only took advantage of this in tests. Change-Id: I2c00f6b839a693e059fa2ce79abf347dbf83bdd0 --- M src/kudu/consensus/consensus_peers-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/log-test-base.h M src/kudu/consensus/log-test.cc M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/consensus/log_cache-test.cc M src/kudu/consensus/log_index-test.cc M src/kudu/consensus/log_index.cc M src/kudu/consensus/log_index.h M src/kudu/consensus/log_reader.cc M src/kudu/consensus/log_reader.h M src/kudu/consensus/log_util.cc M src/kudu/consensus/log_util.h M src/kudu/consensus/mt-log-test.cc M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/integration-tests/log_verifier.cc M src/kudu/integration-tests/timestamp_advancement-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/tablet_bootstrap.h M src/kudu/tablet/tablet_replica-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_local_replica.cc M src/kudu/tools/tool_action_perf.cc M src/kudu/tools/tool_action_wal.cc M src/kudu/tserver/tablet_copy_source_session-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 31 files changed, 246 insertions(+), 110 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/15082/1 -- To view, visit http://gerrit.cloudera.org:8080/15082 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2c00f6b839a693e059fa2ce79abf347dbf83bdd0 Gerrit-Change-Number: 15082 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] rpc-test-base.h: squelch a warning
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15079 to review the following change. Change subject: rpc-test-base.h: squelch a warning .. rpc-test-base.h: squelch a warning Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 --- M src/kudu/rpc/rpc-test-base.h 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/15079/1 -- To view, visit http://gerrit.cloudera.org:8080/15079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4ae8ebb0cb49e32f08f5c3f0c0f5061b029e0638 Gerrit-Change-Number: 15079 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] tablet copy client: delete WAL data using existing function
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15077 to review the following change. Change subject: tablet copy client: delete WAL data using existing function .. tablet copy client: delete WAL data using existing function I thought this might be needed for correctness (if Log::DeleteOnDiskData did something beyond a recursive deletion), but it's not. Nevertheless, it seems more robust, if DeleteOnDiskData _did_ were changed to do something interesting in the future. Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d --- M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 2 files changed, 9 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/15077/1 -- To view, visit http://gerrit.cloudera.org:8080/15077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic18d11a3d857ab9a29a82d9f7ebe72f33737e25d Gerrit-Change-Number: 15077 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] compression: fix handling of NO COMPRESSION
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15078 to review the following change. Change subject: compression: fix handling of NO_COMPRESSION .. compression: fix handling of NO_COMPRESSION The string values of CompressionType and GetCompressionCodecType() did not agree: the former used NO_COMPRESSION and the latter NONE to indicate the lack of compression. This led to some unnecessary warnings when a stringified CompressionType was fed into GetCompressionCodecType(), as is done in log-test. This patch changes GetCompressionCodecType() to expect NO_COMPRESSION rather than NONE. It shouldn't affect backwards compatibility: if someone really does use NONE (i.e. in a gflag), they'll just get no compression anyway, albeit with the ugly warning. That's not ideal, but the alternative (use NONE in CompressionType) may break backwards compatibility in JSON encoding, and NO_COMPRESSION is the value we use in our public APIs. Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 --- M src/kudu/cfile/cfile_writer.cc M src/kudu/consensus/log-test.cc M src/kudu/consensus/log.cc M src/kudu/integration-tests/disk_reservation-itest.cc M src/kudu/integration-tests/raft_consensus-itest-base.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/util/compression/compression_codec.cc 8 files changed, 13 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/15078/1 -- To view, visit http://gerrit.cloudera.org:8080/15078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I900458b7c7ed4be02906479becaaf60bad379029 Gerrit-Change-Number: 15078 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] file cache: support alternate open modes
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15081 to review the following change. Change subject: file cache: support alternate open modes .. file cache: support alternate open modes Previously, the file cache only supported opening existing files; any new file creation happened out-of-band and then the file reopened via the cache. If we're going to use the file cache for log index chunks, however, we need to support CREATE_OR_OPEN style usage, and doing it in the log index itself is somewhat hairy. This patch modifies the file cache to support all of the modes defined in Env::OpenMode. I tried to ensure that cache operations look and feel like a standard POSIX filesystem, but it's tough to get this right, and I'm sure I missed some corner cases, especially those involving the CREATE_OR_OPEN* modes and concurrent deletes. Thankfully none of our use cases (block managers, log segments, and log index chunks) do that. Change-Id: Ie167302ef85b8e1a40fbb89a7742e2cbb43bcec3 --- M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/file_cache-stress-test.cc M src/kudu/util/file_cache-test.cc M src/kudu/util/file_cache.cc M src/kudu/util/file_cache.h 6 files changed, 240 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15081/1 -- To view, visit http://gerrit.cloudera.org:8080/15081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie167302ef85b8e1a40fbb89a7742e2cbb43bcec3 Gerrit-Change-Number: 15081 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] file cache: evict open fd when descriptor goes out of scope
Hello Alexey Serbin, Andrew Wong, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15080 to review the following change. Change subject: file cache: evict open fd when descriptor goes out of scope .. file cache: evict open fd when descriptor goes out of scope Previously, an open fd remained in cache when its descriptor was destroyed unless the file was marked for deletion. This avoided an extra cache lookup at destruction time and provided faster access to a file if it was closed and then reopened (though that second benefit was irrelevant to the block managers where files were kept open permanently unless they were deleted). With this change, an open fd is forcefully evicted when its descriptor is destroyed. This provides better semantics if the goal is to close a file without deleting it and test that the fd is indeed closed. It also prevents "false positive" cache hits when the user closes the file, renames it, and creates a new file with the old file name. This is an access pattern used by the WAL when a replica fails and it is brought back to life via tablet copy from a healthy server. Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 --- M src/kudu/util/file_cache-test.cc M src/kudu/util/file_cache.cc M src/kudu/util/file_cache.h 3 files changed, 24 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/15080/1 -- To view, visit http://gerrit.cloudera.org:8080/15080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iea5317add630753716ef538cc8a198c9b3547822 Gerrit-Change-Number: 15080 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong
[kudu-CR] [master] Replace hive metastore sasl enabled validator
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15076 ) Change subject: [master] Replace hive_metastore_sasl_enabled validator .. [master] Replace hive_metastore_sasl_enabled validator The hive_metastore_sasl_enabled GROUP_FLAG_VALIDATOR that was added in a76177f was accidentally removed in 4f82a46. The validation is added back in this patch. However, it is validated in RunMasterServer() as opposed to using a GROUP_FLAG_VALIDATOR because the master server can be run from the tools now. Change-Id: I9ecea5b4b4e0d4bdd198241f23bf34ea46ff6945 Reviewed-on: http://gerrit.cloudera.org:8080/15076 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/master/master_runner.cc 1 file changed, 19 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9ecea5b4b4e0d4bdd198241f23bf34ea46ff6945 Gerrit-Change-Number: 15076 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] Replace hive metastore sasl enabled validator
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15076 ) Change subject: [master] Replace hive_metastore_sasl_enabled validator .. Patch Set 1: Code-Review+2 Thanks for catching this! -- To view, visit http://gerrit.cloudera.org:8080/15076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ecea5b4b4e0d4bdd198241f23bf34ea46ff6945 Gerrit-Change-Number: 15076 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 21 Jan 2020 18:05:46 + Gerrit-HasComments: No
[kudu-CR] [master] Replace hive metastore sasl enabled validator
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15076 Change subject: [master] Replace hive_metastore_sasl_enabled validator .. [master] Replace hive_metastore_sasl_enabled validator The hive_metastore_sasl_enabled GROUP_FLAG_VALIDATOR that was added in a76177f was accidentally removed in 4f82a46. The validation is added back in this patch. However, it is validated in RunMasterServer() as opposed to using a GROUP_FLAG_VALIDATOR because the master server can be run from the tools now. Change-Id: I9ecea5b4b4e0d4bdd198241f23bf34ea46ff6945 --- M src/kudu/master/master_runner.cc 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/15076/1 -- To view, visit http://gerrit.cloudera.org:8080/15076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9ecea5b4b4e0d4bdd198241f23bf34ea46ff6945 Gerrit-Change-Number: 15076 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [python] Use positional formatting in Python scripts
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15075 to look at the new patch set (#2). Change subject: [python] Use positional formatting in Python scripts .. [python] Use positional formatting in Python scripts get-job-stats-from-mysql.py is not compatible with Python 2.6.x beause it uses the format function without the positions included. I also updated other calls to format without explicit positions. Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a --- M build-support/iwyu/iwyu_tool.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/graph-metrics.py M src/kudu/scripts/max_skew_estimate.py 5 files changed, 8 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/15075/2 -- To view, visit http://gerrit.cloudera.org:8080/15075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Gerrit-Change-Number: 15075 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [python] Use positional formatting in Python scripts
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15075 Change subject: [python] Use positional formatting in Python scripts .. [python] Use positional formatting in Python scripts get-job-stats-from-mysql.py is not compatible with Python 2.6.x beause it uses the format function without the positions included. I also updated other calls to format without explicit positions. Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a --- M build-support/iwyu/iwyu_tool.py M src/kudu/experiments/merge-test.py M src/kudu/scripts/get-job-stats-from-mysql.py M src/kudu/scripts/max_skew_estimate.py 4 files changed, 7 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/15075/1 -- To view, visit http://gerrit.cloudera.org:8080/15075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0eb17a5b0435e8e78a31124c8d9f3756583cd05a Gerrit-Change-Number: 15075 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
