[kudu-CR](branch-1.15.x) KUDU-3297 fix RPC negotiations with cyrus-sasl-gssapi-2.1.27-5 and newer
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17631 ) Change subject: KUDU-3297 fix RPC negotiations with cyrus-sasl-gssapi-2.1.27-5 and newer .. KUDU-3297 fix RPC negotiations with cyrus-sasl-gssapi-2.1.27-5 and newer It turns out that setting SASL security properties such as the minimum Security Strength Factor (SSF) once sasl_client_start() has already been called isn't working as expected anymore once patch [1] is applied for the cyrus-sasl-gssapi plugin. This patch addresses the issue, moving the call to sasl_setprop(..., SASL_SEC_PROPS, ...) prior to the corresponding call to sasl_client_start() in the client-side negotiation logic for C++ Kudu components. Prior to this patch, GSSAPI-involved scenarios of the negotiation-test and security-itest would fail when running against the GSSAPI plugin with patch [1] applied. With this patch, all scenarios in the negotiation-test and the security-itest pass. I didn't add any extra test scenarios since the already existing test coverage was enough to spot the issue, as it can be seen from above. [1] https://github.com/cyrusimap/cyrus-sasl/pull/603 Change-Id: Ia655356798c753d5a223933cc09a0731018e10af Reviewed-on: http://gerrit.cloudera.org:8080/17619 Reviewed-by: Grant Henke Reviewed-by: Greg Solovyev Tested-by: Kudu Jenkins (cherry picked from commit fff48ea4e5eadd365a85a05a82f66b3eb76d0b0b) Reviewed-on: http://gerrit.cloudera.org:8080/17631 Reviewed-by: Bankim Bhavsar --- M src/kudu/rpc/client_negotiation.cc 1 file changed, 7 insertions(+), 7 deletions(-) Approvals: Kudu Jenkins: Verified Bankim Bhavsar: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: merged Gerrit-Change-Id: Ia655356798c753d5a223933cc09a0731018e10af Gerrit-Change-Number: 17631 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.15.x) KUDU-3297 fix RPC negotiations with cyrus-sasl-gssapi-2.1.27-5 and newer
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17631 ) Change subject: KUDU-3297 fix RPC negotiations with cyrus-sasl-gssapi-2.1.27-5 and newer .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: comment Gerrit-Change-Id: Ia655356798c753d5a223933cc09a0731018e10af Gerrit-Change-Number: 17631 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 19 Jul 2021 21:54:05 + Gerrit-HasComments: No
[kudu-CR](branch-1.15.x) [cmake] Boost to use /dev/[u]random for UUID generation (take 2)
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17699 ) Change subject: [cmake] Boost to use /dev/[u]random for UUID generation (take 2) .. [cmake] Boost to use /dev/[u]random for UUID generation (take 2) Since boost's UUID library is a header-only built dependency, it's not enough adding -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX into b2's cxxflags while building boost in Kudu's thirdparty. This patch addresses the issue, defining the BOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX macro for the preprocessor while building Kudu binaries themselves. This is similar to the treatment of a similar macro for the boost's date_time library (the latter is both a binary and a header dependency). Maybe, the issue might be solved by solely adding the corresponding flag into the b2's 'define' flag, but I decided to proceed the same way as it is done for another macro for the boost library. This is a follow-up to 27418145bc60ceb009c663626a88e57748a85c9f. Change-Id: I5eedb644bd9f1cd7a48254580bc8307053ce6c9f Reviewed-on: http://gerrit.cloudera.org:8080/17687 Reviewed-by: Attila Bukor Tested-by: Alexey Serbin (cherry picked from commit 35b5664f908cd1250c9f01e5dff77b653cfd12b7) Reviewed-on: http://gerrit.cloudera.org:8080/17699 Reviewed-by: Bankim Bhavsar Tested-by: Kudu Jenkins --- M CMakeLists.txt M thirdparty/build-definitions.sh 2 files changed, 9 insertions(+), 2 deletions(-) Approvals: Bankim Bhavsar: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/17699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: merged Gerrit-Change-Id: I5eedb644bd9f1cd7a48254580bc8307053ce6c9f Gerrit-Change-Number: 17699 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.15.x) [cmake] Boost to use /dev/[u]random for UUID generation (take 2)
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17699 ) Change subject: [cmake] Boost to use /dev/[u]random for UUID generation (take 2) .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: comment Gerrit-Change-Id: I5eedb644bd9f1cd7a48254580bc8307053ce6c9f Gerrit-Change-Number: 17699 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 19 Jul 2021 19:28:12 + Gerrit-HasComments: No
[kudu-CR](branch-1.15.x) [cmake] Boost to use /dev/[u]random for UUID generation (take 2)
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17699 Change subject: [cmake] Boost to use /dev/[u]random for UUID generation (take 2) .. [cmake] Boost to use /dev/[u]random for UUID generation (take 2) Since boost's UUID library is a header-only built dependency, it's not enough adding -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX into b2's cxxflags while building boost in Kudu's thirdparty. This patch addresses the issue, defining the BOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX macro for the preprocessor while building Kudu binaries themselves. This is similar to the treatment of a similar macro for the boost's date_time library (the latter is both a binary and a header dependency). Maybe, the issue might be solved by solely adding the corresponding flag into the b2's 'define' flag, but I decided to proceed the same way as it is done for another macro for the boost library. This is a follow-up to 27418145bc60ceb009c663626a88e57748a85c9f. Change-Id: I5eedb644bd9f1cd7a48254580bc8307053ce6c9f Reviewed-on: http://gerrit.cloudera.org:8080/17687 Reviewed-by: Attila Bukor Tested-by: Alexey Serbin (cherry picked from commit 35b5664f908cd1250c9f01e5dff77b653cfd12b7) --- M CMakeLists.txt M thirdparty/build-definitions.sh 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/17699/1 -- To view, visit http://gerrit.cloudera.org:8080/17699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.15.x Gerrit-MessageType: newchange Gerrit-Change-Id: I5eedb644bd9f1cd7a48254580bc8307053ce6c9f Gerrit-Change-Number: 17699 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [test] deflake dynamic multi master-test
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17692 ) Change subject: [test] deflake dynamic_multi_master-test .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/integration-tests/master_hms-itest.cc@749 PS2, Line 749: ASSERT_TRUE(s.IsTimedOut()) << s.ToString(); This looks like an unrelated change. http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/master/dynamic_multi_master-test.cc File src/kudu/master/dynamic_multi_master-test.cc: http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/master/dynamic_multi_master-test.cc@223 PS2, Line 223: master.registration().rpc_addresses_size() != 1 Why is having multiple master registration addresses a problem? http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/17692/2/src/kudu/mini-cluster/external_mini_cluster.cc@734 PS2, Line 734: for (auto iter = masters_to_search.begin(); iter != masters_to_search.end(); iter++) for-range loop would work fine for this case. -- To view, visit http://gerrit.cloudera.org:8080/17692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67c5ac38701c4283611dc95e17f9f054c90aba99 Gerrit-Change-Number: 17692 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 19 Jul 2021 16:46:06 + Gerrit-HasComments: Yes
[kudu-CR] [master] make automatic AddMaster resilient to leadership changes
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17691 ) Change subject: [master] make automatic AddMaster resilient to leadership changes .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17691 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie38453c6fc41ce98c59c010902e2d9fe9db62dee Gerrit-Change-Number: 17691 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 19 Jul 2021 16:22:16 + Gerrit-HasComments: No
[kudu-CR] [rest] add rest implementation
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/17555 )
Change subject: [rest] add rest implementation
..
Patch Set 38:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/CMakeLists.txt
File src/kudu/rest/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/CMakeLists.txt@23
PS38, Line 23: add_library(rest controller.h
I believe headers don't need to be added here.
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h
File src/kudu/rest/controller.h:
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h@77
PS38, Line 77: info->addResponse(Status::CODE_400, "text/plain");
shouldn't error handling also be JSON? does Oat++ support that?
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h@85
PS38, Line 85: info->addResponse(Status::CODE_200, "text/plain");
why are the success responses here and below text/plain? Also, maybe we could
use status code 201 here and 204 below if there's no content?
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h@88
PS38, Line 88: CreateKuduTable
nit: missing space after comma here and below
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h@97
PS38, Line 97: ENDPOINT("PUT","/AlterKuduTableName",AlterKuduTableName,
maybe it should be called RenameTable?
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.h@114
PS38, Line 114:
client::sp::shared_ptr* client);
nit: alignment is off here and below
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc
File src/kudu/rest/controller.cc:
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@68
PS38, Line 68: Controller::Controller(const
std::shared_ptr& object_mapper,
nit: this and some other lines are too long - it's okay to break line after "<"
for example
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@143
PS38, Line 143: string name;
Why do these variables exist? Can't the DTO variables be set directly?
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@153
PS38, Line 153: name = kudu_schema.Column(i).name();
maybe _schema.Column(i) can be assigned to a local variable to avoid
having to call Column() multiple times
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@176
PS38, Line 176:
.default_admin_operation_timeout(MonoDelta::FromSeconds(20))
why is this specified here?
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@184
PS38, Line 184: KuduTableCreator* table_creator = client->NewTableCreator();
you could use a smart pointer to avoid having to manually delete it, then you
can simply return Create().
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@198
PS38, Line 198: string name;
same as above
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/controller.cc@213
PS38, Line 213: if (isPrimaryKey) {
you can simplify this by doing auto c =
b.AddColumn(kudu_column_dto->columnName->std_str()), then if
(kudu_column_dto->isPrimaryKey) c->PrimaryKey() etc.
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/rest_server.cc
File src/kudu/rest/rest_server.cc:
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/rest_server.cc@72
PS38, Line 72: .setVersion("1.0")
I think it should use the Kudu version instead of a hardcoded 1.0.
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/rest_server.cc@106
PS38, Line 106: ParseString
a better function name would be ParseBindAddress, or something that tells more
about what this function does.
http://gerrit.cloudera.org:8080/#/c/17555/38/src/kudu/rest/rest_server.cc@107
PS38, Line 107: std::pair p =
strings::Split(rest_bind_address, strings::delimiter::Limit(":", 1));
nit: line too long
--
To view, visit http://gerrit.cloudera.org:8080/17555
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ca3121fd7e95a1267853be45cb5f5855298c763
Gerrit-Change-Number: 17555
Gerrit-PatchSet: 38
Gerrit-Owner: Khazar Mammadli
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 19 Jul 2021 08:01:02 +
Gerrit-HasComments: Yes
[kudu-CR] [rest] add rest to master servers
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17556 ) Change subject: [rest] add rest to master servers .. Patch Set 28: (3 comments) http://gerrit.cloudera.org:8080/#/c/17556/28//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17556/28//COMMIT_MSG@11 PS28, Line 11: nit: extra space http://gerrit.cloudera.org:8080/#/c/17556/28//COMMIT_MSG@14 PS28, Line 14: flags can you add the default port number here? http://gerrit.cloudera.org:8080/#/c/17556/28/src/kudu/master/master.cc File src/kudu/master/master.cc: http://gerrit.cloudera.org:8080/#/c/17556/28/src/kudu/master/master.cc@102 PS28, Line 102: DEFINE_string(rest_server_bind_address,"0.0.0.0:8061","Address where the rest server should start up"); nit: line too long -- To view, visit http://gerrit.cloudera.org:8080/17556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ae85888686dbe3a532f1447aafefb4bc6125ee3 Gerrit-Change-Number: 17556 Gerrit-PatchSet: 28 Gerrit-Owner: Khazar Mammadli Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 19 Jul 2021 08:01:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3189: log Hive Metatore URI when connecting
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17695 to look at the new patch set (#2). Change subject: KUDU-3189: log Hive Metatore URI when connecting .. KUDU-3189: log Hive Metatore URI when connecting When 'kudu hms' is connecting to an HMS instance there is no user facing information log about which HMS instance the connection is opened for. This change adds a simple log line to the thrift client that shows this information, but only when the verbosity level is set to 1. Change-Id: Ide920bc1fca44e318d3c4e2921eb0588a82b7400 --- M src/kudu/thrift/client.h M src/kudu/tools/kudu-tool-test.cc 2 files changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/17695/2 -- To view, visit http://gerrit.cloudera.org:8080/17695 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ide920bc1fca44e318d3c4e2921eb0588a82b7400 Gerrit-Change-Number: 17695 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
