[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16824 Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Revert "[thirdparty] Upgrade glog to 0.4.0" This reverts commit 2943aa701ee092158c2084c614a91f92513ef7c4. We are still seeing errors in fresh builds on Centos 7 that look like below with this patch: [ 6%] Running C++ protocol buffer compiler on hash.proto --insertions_out: protoc-gen-insertions: Plugin killed by signal 11. make[2]: *** [src/kudu/util/hash.pb.cc] Error 1 make[1]: *** [src/kudu/util/CMakeFiles/kudu_util_hash_proto.dir/all] Error 2 Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 --- M src/kudu/fs/dir_util-test.cc M src/kudu/util/logging.h M src/kudu/util/rw_mutex-test.cc M thirdparty/build-definitions.sh M thirdparty/download-thirdparty.sh A thirdparty/patches/glog-fix-symbolization.patch A thirdparty/patches/glog-issue-198-fix-unused-warnings.patch A thirdparty/patches/glog-issue-54-dont-build-tests.patch M thirdparty/patches/glog-support-stacktrace-for-aarch64.patch M thirdparty/vars.sh 10 files changed, 455 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/16824/1 -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Grant Henke has removed a vote on this change. Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16824 ) Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 07 Dec 2020 17:08:54 + Gerrit-HasComments: No
[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16824 ) Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 07 Dec 2020 17:38:23 + Gerrit-HasComments: No
[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16824 ) Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Patch Set 1: Code-Review+2 It seems we had all the necessary functionality with those extra patches for previous version, so I guess it's OK to keep going with the latter for some time. -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 07 Dec 2020 17:58:43 + Gerrit-HasComments: No
[kudu-CR] Revert "[thirdparty] Upgrade glog to 0.4.0"
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16824 ) Change subject: Revert "[thirdparty] Upgrade glog to 0.4.0" .. Revert "[thirdparty] Upgrade glog to 0.4.0" This reverts commit 2943aa701ee092158c2084c614a91f92513ef7c4. We are still seeing errors in fresh builds on Centos 7 that look like below with this patch: [ 6%] Running C++ protocol buffer compiler on hash.proto --insertions_out: protoc-gen-insertions: Plugin killed by signal 11. make[2]: *** [src/kudu/util/hash.pb.cc] Error 1 make[1]: *** [src/kudu/util/CMakeFiles/kudu_util_hash_proto.dir/all] Error 2 Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Reviewed-on: http://gerrit.cloudera.org:8080/16824 Tested-by: Grant Henke Reviewed-by: Bankim Bhavsar Reviewed-by: Alexey Serbin --- M src/kudu/fs/dir_util-test.cc M src/kudu/util/logging.h M src/kudu/util/rw_mutex-test.cc M thirdparty/build-definitions.sh M thirdparty/download-thirdparty.sh A thirdparty/patches/glog-fix-symbolization.patch A thirdparty/patches/glog-issue-198-fix-unused-warnings.patch A thirdparty/patches/glog-issue-54-dont-build-tests.patch M thirdparty/patches/glog-support-stacktrace-for-aarch64.patch M thirdparty/vars.sh 10 files changed, 455 insertions(+), 43 deletions(-) Approvals: Grant Henke: Verified Bankim Bhavsar: Looks good to me, approved Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5635d1e979be45395d0babdb8c07a234a97a0970 Gerrit-Change-Number: 16824 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Replace boost::iequals with our own implementation
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16826 Change subject: Replace boost::iequals with our own implementation .. Replace boost::iequals with our own implementation This patch replaces uses of boost::iequals with our own implementation. This is a step in our ongoing work to reduce dependence on boost. Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 --- M src/kudu/clock/hybrid_clock.cc M src/kudu/hms/hms_client.cc M src/kudu/integration-tests/hms_itest-base.cc M src/kudu/rpc/sasl_common.cc M src/kudu/security/tls_context.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/server_base.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/util/flags.cc M src/kudu/util/string_case-test.cc M src/kudu/util/string_case.cc M src/kudu/util/string_case.h 21 files changed, 141 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16826/1 -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [docs] Copy 1.12.0 release notes to prior release notes
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16827 Change subject: [docs] Copy 1.12.0 release notes to prior release notes .. [docs] Copy 1.12.0 release notes to prior release notes It looks like the 1.12.0 release notes were not copied over to prior_release_notes.adoc during the 1.13.0 release. This patch copies them over. Change-Id: I6d333b1ab21f89fbefc1cb0469df61da28224989 --- M docs/prior_release_notes.adoc 1 file changed, 277 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/16827/1 -- To view, visit http://gerrit.cloudera.org:8080/16827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6d333b1ab21f89fbefc1cb0469df61da28224989 Gerrit-Change-Number: 16827 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [docs] Copy 1.12.0 release notes to prior release notes
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16827 ) Change subject: [docs] Copy 1.12.0 release notes to prior release notes .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d333b1ab21f89fbefc1cb0469df61da28224989 Gerrit-Change-Number: 16827 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 07 Dec 2020 19:35:26 + Gerrit-HasComments: No
[kudu-CR] Replace boost::iequals with our own implementation
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16826 to look at the new patch set (#2). Change subject: Replace boost::iequals with our own implementation .. Replace boost::iequals with our own implementation This patch replaces uses of boost::iequals with our own implementation. This is a step in our ongoing work to reduce dependence on boost. Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 --- M src/kudu/clock/hybrid_clock.cc M src/kudu/hms/hms_client.cc M src/kudu/integration-tests/hms_itest-base.cc M src/kudu/rpc/sasl_common.cc M src/kudu/security/tls_context.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/server_base.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/util/flags.cc M src/kudu/util/string_case-test.cc M src/kudu/util/string_case.cc M src/kudu/util/string_case.h 21 files changed, 141 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16826/2 -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docs] Copy 1.12.0 release notes to prior release notes
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16827 ) Change subject: [docs] Copy 1.12.0 release notes to prior release notes .. [docs] Copy 1.12.0 release notes to prior release notes It looks like the 1.12.0 release notes were not copied over to prior_release_notes.adoc during the 1.13.0 release. This patch copies them over. Change-Id: I6d333b1ab21f89fbefc1cb0469df61da28224989 Reviewed-on: http://gerrit.cloudera.org:8080/16827 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M docs/prior_release_notes.adoc 1 file changed, 277 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6d333b1ab21f89fbefc1cb0469df61da28224989 Gerrit-Change-Number: 16827 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Replace boost::iequals with our own implementation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16826 ) Change subject: Replace boost::iequals with our own implementation .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@7 PS2, Line 7: our own implementation Do we need to have the new function locale-aware like boost::iequals? https://www.boost.org/doc/libs/1_60_0/doc/html/boost/algorithm/iequals.html http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@9 PS2, Line 9: boost::iequals with our own implementation Could you add a test to assess the performance of the new implementation compared with boost::iequals? -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 07 Dec 2020 20:51:32 + Gerrit-HasComments: Yes
[kudu-CR] Replace boost::iequals with our own implementation
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16826 ) Change subject: Replace boost::iequals with our own implementation .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case-test.cc File src/kudu/util/string_case-test.cc: http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case-test.cc@78 PS2, Line 78: nit: only remaining combination that isn't tested is 'mix' and 'capital'. http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc File src/kudu/util/string_case.cc: http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@87 PS2, Line 87: if (b.size() != a.size()) nit: from a consistency point of view is it better to include curly braces for this if statement? Or moving the return statement to the same line as the condition. -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Mon, 07 Dec 2020 22:26:27 + Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Publish docs changes from master
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16828 Change subject: Publish docs changes from master .. Publish docs changes from master This patch publishes a bunch of recent docs changes from the master branch. It also updates the .gitignore file to prevent issues when switching branches. Change-Id: Iedaa57269d77665331fb2846d837ba0f9d544e43 --- M .gitignore M releases/1.13.0/docs/administration.html M releases/1.13.0/docs/background_tasks.html M releases/1.13.0/docs/command_line_tools.html M releases/1.13.0/docs/command_line_tools_reference.html M releases/1.13.0/docs/configuration.html M releases/1.13.0/docs/configuration_reference.html M releases/1.13.0/docs/configuration_reference_unsupported.html M releases/1.13.0/docs/contributing.html M releases/1.13.0/docs/developing.html M releases/1.13.0/docs/export_control.html M releases/1.13.0/docs/hive_metastore.html M releases/1.13.0/docs/index.html M releases/1.13.0/docs/installation.html M releases/1.13.0/docs/known_issues.html M releases/1.13.0/docs/kudu-master_configuration_reference.html M releases/1.13.0/docs/kudu-master_configuration_reference_unsupported.html A releases/1.13.0/docs/kudu-master_metrics_reference.html M releases/1.13.0/docs/kudu-tserver_configuration_reference.html M releases/1.13.0/docs/kudu-tserver_configuration_reference_unsupported.html A releases/1.13.0/docs/kudu-tserver_metrics_reference.html M releases/1.13.0/docs/kudu_impala_integration.html A releases/1.13.0/docs/metrics_reference.html M releases/1.13.0/docs/prior_release_notes.html M releases/1.13.0/docs/quickstart.html M releases/1.13.0/docs/quickstartdev.html M releases/1.13.0/docs/release_notes.html M releases/1.13.0/docs/scaling_guide.html M releases/1.13.0/docs/schema_design.html M releases/1.13.0/docs/security.html M releases/1.13.0/docs/transaction_semantics.html M releases/1.13.0/docs/troubleshooting.html 32 files changed, 40,872 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/16828/1 -- To view, visit http://gerrit.cloudera.org:8080/16828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newchange Gerrit-Change-Id: Iedaa57269d77665331fb2846d837ba0f9d544e43 Gerrit-Change-Number: 16828 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR](gh-pages) Publish docs changes from master
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16828 ) Change subject: Publish docs changes from master .. Patch Set 1: Code-Review+1 (1 comment) I looked through the metrics pages in chrome without rendering on and they look fine. http://gerrit.cloudera.org:8080/#/c/16828/1/releases/1.13.0/docs/installation.html File releases/1.13.0/docs/installation.html: http://gerrit.cloudera.org:8080/#/c/16828/1/releases/1.13.0/docs/installation.html@172 PS1, Line 172: 7.6 Not an issue with this patch specifically, but this is odd because one of our dev boxes is 7.3.1611 and builds fine. Is this supposed to reference a different version? Or am I misunderstanding what this is referring to? -- To view, visit http://gerrit.cloudera.org:8080/16828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iedaa57269d77665331fb2846d837ba0f9d544e43 Gerrit-Change-Number: 16828 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Comment-Date: Mon, 07 Dec 2020 22:49:38 + Gerrit-HasComments: Yes
[kudu-CR] Replace boost::iequals with our own implementation
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16826 to look at the new patch set (#3). Change subject: Replace boost::iequals with our own implementation .. Replace boost::iequals with our own implementation This patch replaces uses of boost::iequals with our own implementation. This is a step in our ongoing work to reduce dependence on boost. Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 --- M src/kudu/clock/hybrid_clock.cc M src/kudu/hms/hms_client.cc M src/kudu/integration-tests/hms_itest-base.cc M src/kudu/rpc/sasl_common.cc M src/kudu/security/tls_context.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/server_base.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/util/flags.cc M src/kudu/util/string_case-test.cc M src/kudu/util/string_case.cc M src/kudu/util/string_case.h 21 files changed, 143 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16826/3 -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] Replace boost::iequals with our own implementation
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/16826 )
Change subject: Replace boost::iequals with our own implementation
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@7
PS2, Line 7: our own implementation
> Do we need to have the new function locale-aware like boost::iequals?
We don't need locale aware. All usage today is against known no-locale specific
string values (flags, configs, tests, etc).
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@9
PS2, Line 9: boost::iequals with our own implementation
> Could you add a test to assess the performance of the new implementation co
All usage is on non-performant sensitive code paths (flags, configs, tests,
etc).
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case-test.cc
File src/kudu/util/string_case-test.cc:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case-test.cc@78
PS2, Line 78: ASSERT_TRUE(iequals(caps, mix));
> nit: only remaining combination that isn't tested is 'mix' and 'capital'.
Done
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc
File src/kudu/util/string_case.cc:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@87
PS2, Line 87: if (b.size() != a.size()) {
> nit: from a consistency point of view is it better to include curly braces
Done
--
To view, visit http://gerrit.cloudera.org:8080/16826
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233
Gerrit-Change-Number: 16826
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Comment-Date: Mon, 07 Dec 2020 22:58:59 +
Gerrit-HasComments: Yes
[kudu-CR] Replace boost::iequals with our own implementation
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16826 )
Change subject: Replace boost::iequals with our own implementation
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@7
PS2, Line 7: our own implementation
> We don't need locale aware. All usage today is against known no-locale spec
Cool, thank you for the clarification. Does it make sense to reflect this fact
in the commit description as well?
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@9
PS2, Line 9: boost::iequals with our own implementation
> All usage is on non-performant sensitive code paths (flags, configs, tests,
Ah, indeed: makes sense.
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h
File src/kudu/util/string_case.h:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h@61
PS2, Line 61: iequals
Since it's targeted to work as expected only with ASCII symbols, maybe rename
this into ascii_iequals()?
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc
File src/kudu/util/string_case.cc:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@86
PS2, Line 86: bool iequals(const string& a, const string& b) {
Since it's targeted to work only with ASCII characters, does it make sense to
add
DCHECK(IsAscii(a));
DCHECK(IsAscii(b));
?
--
To view, visit http://gerrit.cloudera.org:8080/16826
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233
Gerrit-Change-Number: 16826
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Comment-Date: Mon, 07 Dec 2020 23:52:00 +
Gerrit-HasComments: Yes
[kudu-CR] tablet: allow interleaving of row liveness between compaction input rows
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16752 ) Change subject: tablet: allow interleaving of row liveness between compaction input rows .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@318 PS2, Line 318: // UNDO(del@5) <- BASE(a) -> REDO(del@10) -> REDO(reins@25) // main MRS : // UNDO(del@15) <- BASE(a) -> REDO(del@20) // txn MRS Wondering what will happen if UNDO(del@5) <- BASE(a) -> REDO(del@10) -> REDO(reins@25) ->REDO(del@35) // main MRS UNDO(del@15) <- BASE(a) -> REDO(del@20) -> REDO(reins@40) ->REDO(del@45) // txn MRS it will be transferred to the following? UNDO(del@15) <- BASE(a) -> REDO(del@20) -> REDO(reins@25) ->REDO(del@35)-> REDO(reins@40) ->REDO(del@45)// txn MRS UNDO(del@5) <- BASE(a) -> REDO(del@10) // main MRS http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@390 PS2, Line 390: For instance this is a valid history with non-disjoint REDO histories Just for my own understanding, wondering when this happen if all the ops are not in the same txn? http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@453 PS2, Line 453: TransferRedoHistory It seems for all REDOs in the 'older' that are greater than the REDO head of 'newer' will be transferred to the end of the 'newer' REDO. What if there is a overlapped between the transferred REDOs and the 'newer' REDO? e.g. UNDO(del@5) <- BASE(a) -> REDO(del@10) -> REDO(reins@25) ->REDO(del@35) // main MRS UNDO(del@15) <- BASE(a) -> REDO(del@20) -> REDO(reins@40) ->REDO(del@45) // txn MRS will be transferred to the following? UNDO(del@15) <- BASE(a) -> REDO(del@20) -> REDO(reins@40) ->REDO(del@45) -> REDO(reins@25) ->REDO(del@35) // txn MRS Do you think it is a valid example?Will it matter if the 'newer' REDO is out of sequence? -- To view, visit http://gerrit.cloudera.org:8080/16752 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I042a7d70d32edf9d2a3a077790821893f162880a Gerrit-Change-Number: 16752 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 08 Dec 2020 01:00:05 + Gerrit-HasComments: Yes
[kudu-CR] [master][consensus] Procedure for copying system catalog
Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16830 Change subject: [master][consensus] Procedure for copying system catalog .. [master][consensus] Procedure for copying system catalog This change outlines procedure to copy system catalog for the newly added master using existing CLI tools and the master ChangeConfig RPC. - Flag --consensus_allow_status_msg_for_failed_peer must be turned on for existing masters. - Start the new master with --master_address_add_new_master= and --master_addresses that contains itself and existing masters. - Invoke ChangeConfig to add the master. - If the new master is promoted to being a VOTER then following tablet copy steps can be skipped. - Shutdown the new master. - Delete the system catalog on the new master. - Copy the system catalog from the leader master to the new master. - Bring up the new master. - Verify the new master is promoted as VOTER. Bulk of the change is in the test code and involves refactoring to use common parts of the earlier postive test case. One functional change is in Raft consensus and adds the ability for the leader to send status only messages to the peer even if it's in FAILED_UNRECOVERABLE state. Without this change when the system catalog is copied externally the new master remains in FAILED_UNRECOVERABLE state and doesn't get promoted to being a VOTER despite the system catalog being up to date. This behavior is currently disabled by default and hidden under the --consensus_allow_status_msg_for_failed_peer flag. Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 (cherry picked from commit 0d97614c04a072391a643b54220ee1ac5a52a7d4) --- M src/kudu/consensus/consensus_queue.cc M src/kudu/master/dynamic_multi_master-test.cc 2 files changed, 262 insertions(+), 130 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/16830/1 -- To view, visit http://gerrit.cloudera.org:8080/16830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 Gerrit-Change-Number: 16830 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/16815 )
Change subject: [txn] update not-a-leader retry logic in TxnSystemClient
..
Patch Set 2:
(1 comment)
Overall looks good to me.
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc
File src/kudu/transactions/coordinator_rpc.cc:
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc@188
PS2, Line 188: if (result.status.IsAborted()) {
> Should we also be handling IllegalState? E.g. if we change leaders as we ar
+1
--
To view, visit http://gerrit.cloudera.org:8080/16815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59
Gerrit-Change-Number: 16815
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 01:19:02 +
Gerrit-HasComments: Yes
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Hello Alexey Serbin, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16814 to look at the new patch set (#2). Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. [tserver] KUDU-2612: participant op RPC endpoint This adds an RPC endpoint to the tablet servers that allows proxies to interact with transaction participants. This will be used in step 13 and step 18 of the transaction write path[1], allowing the TxnStatusManagers to update participants' transaction states. [1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x Change-Id: Ic48895438ce67e453d235934ac560efe8415921b --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 4 files changed, 277 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/16814/2 -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16814 )
Change subject: [tserver] KUDU-2612: participant op RPC endpoint
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/16814/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16814/1//COMMIT_MSG@9
PS1, Line 9: This adds an RPC endpoint to the tablet servers that allows
proxies to
: interact with transaction participants.
> Could you add more colors on this, adding an example of a scenario when and
Done
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc
File src/kudu/integration-tests/txn_participant-itest.cc:
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc@521
PS1, Line 521: not RUNNING
> BTW, what is the expected behavior what a tablet is bootstrapping? Is it t
It's the same. Added a test.
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/integration-tests/txn_participant-itest.cc@529
PS1, Line 529: dummy-tablet-id
> BTW, would we allow status tablet to participate in a transaction? Not sur
I don't think it's out of the question, especially since the base functionality
of transactions doesn't rely on transactions, so there is no chicken/egg
bootstrapping problem there.
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto
File src/kudu/tserver/tserver_admin.proto:
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto@113
PS1, Line 113: message ParticipantRequestPB {
: optional bytes tablet_id = 1;
: optional ParticipantOpPB op = 2;
: }
> Just want to double-check it's safe to swap the fields. I guess it's so be
Yeah, these aren't used at all in previous versions. Even if they were, I think
it's acceptable to do these breaking changes provided the feature is
experimental (and even incomplete in 1.13).
http://gerrit.cloudera.org:8080/#/c/16814/1/src/kudu/tserver/tserver_admin.proto@256
PS1, Line 256: Have a tablet participate in a transaction
> It would nice if the comment were a bit more clearer w.r.t. what is the sem
Done
--
To view, visit http://gerrit.cloudera.org:8080/16814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b
Gerrit-Change-Number: 16814
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 02:28:30 +
Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Publish docs changes from master
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16828 ) Change subject: Publish docs changes from master .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16828/1/releases/1.13.0/docs/installation.html File releases/1.13.0/docs/installation.html: http://gerrit.cloudera.org:8080/#/c/16828/1/releases/1.13.0/docs/installation.html@172 PS1, Line 172: 7.6 > Not an issue with this patch specifically, but this is odd because one of o I bet it was a typo going from v6 to v7 and missed dropping the .6 -- To view, visit http://gerrit.cloudera.org:8080/16828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iedaa57269d77665331fb2846d837ba0f9d544e43 Gerrit-Change-Number: 16828 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Comment-Date: Tue, 08 Dec 2020 02:54:29 + Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Publish docs changes from master
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16828 ) Change subject: Publish docs changes from master .. Patch Set 1: I will make some more docs changes and update this patch -- To view, visit http://gerrit.cloudera.org:8080/16828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iedaa57269d77665331fb2846d837ba0f9d544e43 Gerrit-Change-Number: 16828 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Comment-Date: Tue, 08 Dec 2020 02:54:43 + Gerrit-HasComments: No
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16815 )
Change subject: [txn] update not-a-leader retry logic in TxnSystemClient
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc
File src/kudu/transactions/coordinator_rpc.cc:
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc@188
PS2, Line 188: if (result.status.IsAborted()) {
> +1
I guess that was supposed to be handled in the code below with
TabletServerErrorPB::NOT_THE_LEADER.
Handling IllegalState() was a major point of pain here because TxnStatusManager
returns IllegalState() if transaction is not in right state, and just assume
that that's the leader change is not correct.
It seems I need to clarify on this: probably, that means we'll need to
introduce error codes specific to TxnStatusManager interface.
--
To view, visit http://gerrit.cloudera.org:8080/16815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59
Gerrit-Change-Number: 16815
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 02:59:58 +
Gerrit-HasComments: Yes
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16815 )
Change subject: [txn] update not-a-leader retry logic in TxnSystemClient
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc
File src/kudu/transactions/coordinator_rpc.cc:
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc@188
PS2, Line 188: if (result.status.IsAborted()) {
> I guess that was supposed to be handled in the code below with TabletServer
Right, I am running into similar issues on the TxnParticipant side. I think one
approach would be to unwrap resp.error().code() early, selecting an appropriate
result as below, and if not, then check the error status, the observation being
that error codes are much more fine-grained than Statuses. E.g. for the
TxnStatusManager, in addition to returning IllegalState(), we could set
ts_error to TXN_ILLEGAL_STATE.
--
To view, visit http://gerrit.cloudera.org:8080/16815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59
Gerrit-Change-Number: 16815
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 03:22:47 +
Gerrit-HasComments: Yes
[kudu-CR] Replace boost::iequals with our own implementation
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16826 to look at the new patch set (#4). Change subject: Replace boost::iequals with our own implementation .. Replace boost::iequals with our own implementation This patch replaces uses of boost::iequals with our own implementation. This is a step in our ongoing work to reduce dependence on boost. Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 --- M src/kudu/clock/hybrid_clock.cc M src/kudu/hms/hms_client.cc M src/kudu/integration-tests/hms_itest-base.cc M src/kudu/rpc/sasl_common.cc M src/kudu/security/tls_context.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/server_base.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/util/flags.cc M src/kudu/util/string_case-test.cc M src/kudu/util/string_case.cc M src/kudu/util/string_case.h 21 files changed, 158 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16826/4 -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] Replace boost::iequals with our own implementation
Hello Mahesh Reddy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16826 to look at the new patch set (#5). Change subject: Replace boost::iequals with our own implementation .. Replace boost::iequals with our own implementation This patch replaces uses of boost::iequals with our own implementation. This is a step in our ongoing work to reduce dependence on boost. Note that we don’t need to worry about locale awareness or performance given all usage today is against ascii strings (not locale specific) and are not in performance sensitive code paths. Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 --- M src/kudu/clock/hybrid_clock.cc M src/kudu/hms/hms_client.cc M src/kudu/integration-tests/hms_itest-base.cc M src/kudu/rpc/sasl_common.cc M src/kudu/security/tls_context.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/server_base.cc M src/kudu/tools/ksck.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_hms.cc M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_action_tserver.cc M src/kudu/util/flags.cc M src/kudu/util/string_case-test.cc M src/kudu/util/string_case.cc M src/kudu/util/string_case.h 21 files changed, 158 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16826/5 -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] Replace boost::iequals with our own implementation
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/16826 )
Change subject: Replace boost::iequals with our own implementation
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16826/2//COMMIT_MSG@7
PS2, Line 7: our own implementation
> Cool, thank you for the clarification. Does it make sense to reflect this
Done
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h
File src/kudu/util/string_case.h:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.h@61
PS2, Line 61: iequals
> Since it's targeted to work as expected only with ASCII symbols, maybe rena
I like the idea of adding the DCHECK but keeping the name the same. The other
methods in this class also expect ascii but do not add that to the name. In the
future we can enhance the impl if needed.
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc
File src/kudu/util/string_case.cc:
http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@86
PS2, Line 86: bool iequals(const string& a, const string& b) {
> Since it's targeted to work only with ASCII characters, does it make sense
good idea. I added these checks to the other methods as well.
--
To view, visit http://gerrit.cloudera.org:8080/16826
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233
Gerrit-Change-Number: 16826
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Comment-Date: Tue, 08 Dec 2020 03:46:41 +
Gerrit-HasComments: Yes
[kudu-CR] Replace boost::iequals with our own implementation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16826 ) Change subject: Replace boost::iequals with our own implementation .. Patch Set 5: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/16826/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16826/5//COMMIT_MSG@13 PS5, Line 13: ascii nit: ASCII http://gerrit.cloudera.org:8080/#/c/16826/5/src/kudu/util/string_case-test.cc File src/kudu/util/string_case-test.cc: http://gerrit.cloudera.org:8080/#/c/16826/5/src/kudu/util/string_case-test.cc@66 PS5, Line 66: string foo = "foo"; : string capital = "Foo"; : string caps = "FOO"; : string mix = "FoO"; : string zero = "FO0"; nit: if there is an idea to test for 'const' qualifier of the parameters of iequals() as well, maybe make these 'const string' http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc File src/kudu/util/string_case.cc: http://gerrit.cloudera.org:8080/#/c/16826/2/src/kudu/util/string_case.cc@86 PS2, Line 86: DCHECK(IsAscii(*word)); > good idea. I added these checks to the other methods as well. Cool, thanks! -- To view, visit http://gerrit.cloudera.org:8080/16826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d2fdd30b739cee53e1551f784dbebfbe76e9233 Gerrit-Change-Number: 16826 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 08 Dec 2020 04:04:35 + Gerrit-HasComments: Yes
[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16831 Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors .. txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors I have a patch coming up that sends RPCs that submit participant ops to the TxnParticipant and retries if necessary. One of the criteria to retry an RPC is checking if the Raft application returned an IllegalState error, e.g. in RaftConsensus::CheckLeadershipAndBindTerm. This is problematic, as TxnParticipants may also return IllegalState if the transaction is not in an appropriate state (e.g. an RPC tries to abort an already-committed transaction). To disambiguate such cases, this patch updates the TxnParticipants to also set a TabletServerErrorPB code when such an error occurs, allowing for more specific error handling to be determined. Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tablet/ops/participant_op.cc M src/kudu/tablet/ops/participant_op.h M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.h 5 files changed, 157 insertions(+), 88 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16831/1 -- To view, visit http://gerrit.cloudera.org:8080/16831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf Gerrit-Change-Number: 16831 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Hello Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16815 to look at the new patch set (#3). Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. [txn] update not-a-leader retry logic in TxnSystemClient This patch updates the retry logic in TxnSystemClient to handle the case when TxnStatusManager returns Status::ServiceUnavailable(). In addition, this patch adds extra provision to check for tablet replica status in StatusManager::CheckTxnStatusDataLoadedUnlocked(). The signatures of the involved methods have been updated as well. This patch also enhances FinalizeCommitTransaction() to report on errors from underlying tablet, if any. I didn't add new tests scenarios in this patch: they are present in follow-up patches which introduce the tracking of stalled transactions. Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 --- M src/kudu/client/client-test.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/coordinator_rpc.cc M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/tserver/tablet_service.cc 7 files changed, 175 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/16815/3 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16815 )
Change subject: [txn] update not-a-leader retry logic in TxnSystemClient
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc
File src/kudu/transactions/coordinator_rpc.cc:
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc@188
PS2, Line 188: if (result.status.IsAborted()) {
> Right, I am running into similar issues on the TxnParticipant side. I think
OK, I added corresponding code into TxnStatusManager.
http://gerrit.cloudera.org:8080/#/c/16815/2/src/kudu/transactions/coordinator_rpc.cc@194
PS2, Line 194: TABLET_NOT_FOUND
> nit: maybe move this comment down by TABLET_NOT_FOUND?
Done
--
To view, visit http://gerrit.cloudera.org:8080/16815
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59
Gerrit-Change-Number: 16815
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Dec 2020 05:04:28 +
Gerrit-HasComments: Yes
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16815/3/src/kudu/transactions/coordinator_rpc.cc File src/kudu/transactions/coordinator_rpc.cc: http://gerrit.cloudera.org:8080/#/c/16815/3/src/kudu/transactions/coordinator_rpc.cc@211 PS3, Line 211: DLOG(FATAL) << "unhandled code: " << TabletServerErrorPB::Code_Name(code); Should we fall through to the more general error handling code below? E.g. for UNKNOWN_ERRORs? -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 05:21:07 + Gerrit-HasComments: Yes
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Hello Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16815 to look at the new patch set (#4). Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. [txn] update not-a-leader retry logic in TxnSystemClient This patch updates the retry logic in TxnSystemClient to handle the case when TxnStatusManager returns Status::ServiceUnavailable(). In addition, this patch adds extra provision to check for tablet replica status in StatusManager::CheckTxnStatusDataLoadedUnlocked(). The signatures of the involved methods have been updated as well. This patch also enhances FinalizeCommitTransaction() to report on errors from underlying tablet, if any. I didn't add new tests scenarios in this patch: they are present in follow-up patches which introduce the tracking of stalled transactions. Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 --- M src/kudu/client/client-test.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/coordinator_rpc.cc M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/tserver/tablet_service.cc 7 files changed, 179 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/16815/4 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16815/3/src/kudu/transactions/coordinator_rpc.cc File src/kudu/transactions/coordinator_rpc.cc: http://gerrit.cloudera.org:8080/#/c/16815/3/src/kudu/transactions/coordinator_rpc.cc@211 PS3, Line 211: DLOG(FATAL) << "unhandled code: " << TabletServerErrorPB::Code_Name(code); > Should we fall through to the more general error handling code below? E.g. Good catch! Done. -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:02:50 + Gerrit-HasComments: Yes
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:06:38 + Gerrit-HasComments: No
[kudu-CR] [master][consensus] Procedure for copying system catalog
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16830 to look at the new patch set (#2). Change subject: [master][consensus] Procedure for copying system catalog .. [master][consensus] Procedure for copying system catalog This change outlines procedure to copy system catalog for the newlyadded master using existing CLI tools and the master ChangeConfig RPC. - Flag --consensus_allow_status_msg_for_failed_peer must be turned on for existing masters. - Start the new master with --master_address_add_new_master= and --master_addresses that contains itself and existing masters. - Invoke ChangeConfig to add the master. - If the new master is promoted to being a VOTER then following tablet copy steps can be skipped. - Shutdown the new master. - Delete the system catalog on the new master. - Copy the system catalog from the leader master to the new master. - Bring up the new master. - Verify the new master is promoted as VOTER. Bulk of the change is in the test code and involves refactoring to use common parts of the earlier postive test case. One functional change is in Raft consensus and adds the ability for the leader to send status only messages to the peer even if it's in FAILED_UNRECOVERABLE state. Without this change when the system catalog is copied externally the new master remains in FAILED_UNRECOVERABLE state and doesn't get promoted to being a VOTER despite the system catalog being up to date. This behavior is currently disabled by default and hidden under the --consensus_allow_status_msg_for_failed_peer flag. Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 --- M src/kudu/consensus/consensus_queue.cc M src/kudu/master/dynamic_multi_master-test.cc 2 files changed, 262 insertions(+), 130 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/16830/2 -- To view, visit http://gerrit.cloudera.org:8080/16830 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I142c1dec442ec72c38c5be9d62cdf270e441d6e3 Gerrit-Change-Number: 16830 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Hello Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16815 to look at the new patch set (#5). Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. [txn] update not-a-leader retry logic in TxnSystemClient This patch updates the retry logic in TxnSystemClient to handle the case when TxnStatusManager returns Status::ServiceUnavailable(). In addition, this patch adds extra provision to check for tablet replica status in StatusManager::CheckTxnStatusDataLoadedUnlocked(). The signatures of the involved methods have been updated as well. This patch also enhances FinalizeCommitTransaction() to report on errors from underlying tablet, if any. Also, TXN_ILLEGAL_STATE error code is added into CoordinateTransactionResponsePB responses when TxnStatusManager finds the requested transaction in a wrong state. I didn't add new tests scenarios in this patch: they are present in follow-up patches which introduce the tracking of stalled transactions. Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 --- M src/kudu/client/client-test.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/coordinator_rpc.cc M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/tserver/tablet_service.cc 7 files changed, 178 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/16815/5 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 ) Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf Gerrit-Change-Number: 16831 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:34:44 + Gerrit-HasComments: No
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:34:57 + Gerrit-HasComments: No
[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 ) Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors .. Patch Set 1: It seems IWYU isn't yet happy, but otherwise LGTM. -- To view, visit http://gerrit.cloudera.org:8080/16831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf Gerrit-Change-Number: 16831 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:35:25 + Gerrit-HasComments: No
[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16831 to look at the new patch set (#2). Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors .. txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors I have a patch coming up that sends RPCs that submit participant ops to the TxnParticipant and retries if necessary. One of the criteria to retry an RPC is checking if the Raft application returned an IllegalState error, e.g. in RaftConsensus::CheckLeadershipAndBindTerm. This is problematic, as TxnParticipants may also return IllegalState if the transaction is not in an appropriate state (e.g. an RPC tries to abort an already-committed transaction). To disambiguate such cases, this patch updates the TxnParticipants to also set a TabletServerErrorPB code when such an error occurs, allowing for more specific error handling to be determined. Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tablet/ops/participant_op.cc M src/kudu/tablet/ops/participant_op.h M src/kudu/tablet/txn_participant-test.cc M src/kudu/tablet/txn_participant.h 5 files changed, 159 insertions(+), 88 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16831/2 -- To view, visit http://gerrit.cloudera.org:8080/16831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf Gerrit-Change-Number: 16831 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Hello Alexey Serbin, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16814 to look at the new patch set (#3). Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. [tserver] KUDU-2612: participant op RPC endpoint This adds an RPC endpoint to the tablet servers that allows proxies to interact with transaction participants. This will be used in step 13 and step 18 of the transaction write path[1], allowing the TxnStatusManagers to update participants' transaction states. [1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x Change-Id: Ic48895438ce67e453d235934ac560efe8415921b --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 4 files changed, 281 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/16814/3 -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] txn participant: return TXN ILLEGAL STATE code on illegal state errors
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16831 ) Change subject: txn_participant: return TXN_ILLEGAL_STATE code on illegal state errors .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16831 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfb8400666855c694b78b1425b1c121597ec7ccf Gerrit-Change-Number: 16831 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 06:51:25 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/16779/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16779/3//COMMIT_MSG@14 PS3, Line 14: goes : out of scope. In contrast, if the transaction handle is created by : KuduTransaction::Deserialize(), the keepalive messages are or aren't Can it happen that the original txn handle doesn't go out of scope and another node create a txn handle by Deserialize() the original txn handle with keep alive enabled? If possible, do you think there is problem that both sending keepalive message? http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h@485 PS3, Line 485: ring topology Just curious, do you know any applications (that integrate with Kudu or not) are using this model? http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.h File src/kudu/client/transaction-internal.h: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.h@66 PS3, Line 66: The 'status' parameter is to nit: unfinished? -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 08 Dec 2020 07:36:43 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16814 ) Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. Patch Set 3: Code-Review+2 (4 comments) http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc File src/kudu/integration-tests/txn_participant-itest.cc: http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@523 PS3, Line 523: TODO(awong): make repeated, idempotent return OK instead of an error. Yep, I guess it will be more robust that way. http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@622 PS3, Line 622: Status nit for here and in other similar scenarios below: use auto? http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@623 PS3, Line 623: ASSERT_TRUE(resp_error.IsIllegalState()); nit: add << s.ToString() just in case it fails http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@648 PS3, Line 648: ASSERT_TRUE(resp_error.IsIllegalState()); nit: add << resp_error.ToString() -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:36:54 + Gerrit-HasComments: Yes
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:40:19 + Gerrit-HasComments: No
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Patch Set 5: Verified+1 I'll take care of the issue reported by ASAN separately -- it's not exactly related to the retrial logic, as far as I can see. -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:44:05 + Gerrit-HasComments: No
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has removed a vote on this change. Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [txn] update not-a-leader retry logic in TxnSystemClient
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16815 ) Change subject: [txn] update not-a-leader retry logic in TxnSystemClient .. [txn] update not-a-leader retry logic in TxnSystemClient This patch updates the retry logic in TxnSystemClient to handle the case when TxnStatusManager returns Status::ServiceUnavailable(). In addition, this patch adds extra provision to check for tablet replica status in StatusManager::CheckTxnStatusDataLoadedUnlocked(). The signatures of the involved methods have been updated as well. This patch also enhances FinalizeCommitTransaction() to report on errors from underlying tablet, if any. Also, TXN_ILLEGAL_STATE error code is added into CoordinateTransactionResponsePB responses when TxnStatusManager finds the requested transaction in a wrong state. I didn't add new tests scenarios in this patch: they are present in follow-up patches which introduce the tracking of stalled transactions. Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Reviewed-on: http://gerrit.cloudera.org:8080/16815 Reviewed-by: Andrew Wong Reviewed-by: Hao Hao Tested-by: Alexey Serbin --- M src/kudu/client/client-test.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/coordinator_rpc.cc M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/tserver/tablet_service.cc 7 files changed, 178 insertions(+), 74 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Hao Hao: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/16815 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1b8abb27c7678c5c616a325343620902f6cbfd59 Gerrit-Change-Number: 16815 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Hello Alexey Serbin, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16814 to look at the new patch set (#4). Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. [tserver] KUDU-2612: participant op RPC endpoint This adds an RPC endpoint to the tablet servers that allows proxies to interact with transaction participants. This will be used in step 13 and step 18 of the transaction write path[1], allowing the TxnStatusManagers to update participants' transaction states. [1] https://docs.google.com/document/d/1qv7Zejpfzg-HvF5azRL49g5lRLQ4437EmJ53GiupcWQ/edit#heading=h.4lm41o75ev1x Change-Id: Ic48895438ce67e453d235934ac560efe8415921b --- M src/kudu/integration-tests/txn_participant-itest.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/tserver_admin.proto 4 files changed, 281 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/16814/4 -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16814 ) Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc File src/kudu/integration-tests/txn_participant-itest.cc: http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@523 PS3, Line 523: TODO(awong): make repeated, idempotent return OK instead of an error. > Yep, I guess it will be more robust that way. Ack http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@622 PS3, Line 622: auto r > nit for here and in other similar scenarios below: use auto? Done http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@623 PS3, Line 623: ASSERT_TRUE(resp_error.IsIllegalState()) << resp_error.ToString(); > nit: add << s.ToString() just in case it fails Done http://gerrit.cloudera.org:8080/#/c/16814/3/src/kudu/integration-tests/txn_participant-itest.cc@648 PS3, Line 648: ASSERT_TRUE(resp_error.IsIllegalState()) << resp_error.ToString(); > nit: add << resp_error.ToString() Done -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:49:40 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16814 ) Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:51:15 + Gerrit-HasComments: No
[kudu-CR] [tserver] KUDU-2612: participant op RPC endpoint
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16814 ) Change subject: [tserver] KUDU-2612: participant op RPC endpoint .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48895438ce67e453d235934ac560efe8415921b Gerrit-Change-Number: 16814 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Dec 2020 07:51:27 + Gerrit-HasComments: No
