[kudu-CR] [examples] updated README.md for basic-python-example
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12458 ) Change subject: [examples] updated README.md for basic-python-example .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12458/2/examples/python/basic-python-example/README.md File examples/python/basic-python-example/README.md: http://gerrit.cloudera.org:8080/#/c/12458/2/examples/python/basic-python-example/README.md@22 PS2, Line 22: It's assumed the commands below are run from the directory where : this README.md file is located, i.e. from : `$KUDU_HOME/examples/python/basic-python-example` directory. This probably belongs down by "Running the example", right? The commands up here all specify the directory. http://gerrit.cloudera.org:8080/#/c/12458/2/examples/python/basic-python-example/README.md@24 PS2, Line 24: Also, : the recipe requires the Kudu C++ components already built in : `$KUDU_HOME/build/latest`. There's probably a way to merge this with the NOTE below, since it mentions the latest build. -- To view, visit http://gerrit.cloudera.org:8080/12458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I103dde66b69db17099b94d66777df37abb4f9fa8 Gerrit-Change-Number: 12458 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Feb 2019 07:22:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1900: add loopback check and test
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/12474 )
Change subject: KUDU-1900: add loopback check and test
..
Patch Set 5:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12474/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12474/5//COMMIT_MSG@23
PS5, Line 23: unencripted auth token
Seems to be a typo. Also, what is unencrypted auth token? Per my knowledge,
we never encrypt authn/authz tokens themselves.
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc@335
PS5, Line 335: struct ifaddrs *ifap;
: if (getifaddrs(&ifap) > -1) {
: SCOPED_CLEANUP({
: freeifaddrs(ifap);
: });
: for (struct ifaddrs *ifa = ifap; ifa; ifa = ifa->ifa_next) {
: if (ifa->ifa_addr == nullptr || ifa->ifa_netmask == nullptr
: || ifa->ifa_addr->sa_family != AF_INET)
: continue;
:
: struct sockaddr_in *addr_in = reinterpret_cast(ifa->ifa_addr);
: if
((NetworkByteOrder::FromHost32(addr_in->sin_addr.s_addr) >> 24) != 127) {
: char s[INET_ADDRSTRLEN];
: inet_ntop(AF_INET, &(addr_in->sin_addr), s,
INET_ADDRSTRLEN);
: FLAGS_local_ip_for_outbound_sockets = string(s,
arraysize(s));
: // Found and assigned an external IP.
: return true;
: }
: }
: }
Is it possible to call kudu::GetLocalNetworks() and then just work with the
result vector to extract non-loopback addresses? While doing so, feel
free to add new useful functions into src/kudu/util/net/net_util.{h,cc}
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc@375
PS5, Line 375: encrypted
Wait, but what about { BindMode::LOOPBACK, "disabled", "disabled", true,
false, false, } ? That's the case when connections are not encrypted, right?
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc@382
PS5, Line 382: { BindMode::LOOPBACK, "required", "required", false,
false, true, },
: { BindMode::LOOPBACK, "disabled", "required", false,
false, true, },
: { BindMode::LOOPBACK, "disabled", "disabled", false,
false, true, },
When it became 3 boolean parameters, it's harder to read this matrix. Maybe,
define some boolean constants with sound names for better readability and use
them instead of just false/true?
With that a configuration entry would look like:
{ BindMode::LOOPBACK, "required", "required", LOOPBACK_UNENCRYPTED,
CLIENT_IP_EXTERNAL, TOKEN_PRESENT, }
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc@406
PS5, Line 406: true
Is this typo?
http://gerrit.cloudera.org:8080/#/c/12474/5/src/kudu/integration-tests/security-itest.cc@437
PS5, Line 437: if (!assignIPToClient(params.force_external_client_ip)) {
: LOG(WARNING) << "Skipping external connection test, because
the host does "
: "not have an external network interface.";
: return;
: }
Is it possible to make this check before starting cluster (i.e. StartCluster()
at line 429)?
--
To view, visit http://gerrit.cloudera.org:8080/12474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f
Gerrit-Change-Number: 12474
Gerrit-PatchSet: 5
Gerrit-Owner: Greg Solovyev
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Greg Solovyev
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 14 Feb 2019 07:06:12 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1900: add loopback check and test
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12474 to look at the new patch set (#5). Change subject: KUDU-1900: add loopback check and test .. KUDU-1900: add loopback check and test This change modifies Socket::IsLoopbackConnection to check if remote IP is in 127.0.0.0/8 subnet. With this change all connections from 127.0.0.0/8 are treated as local. This change adds force_external_client_ip parameter to AuthTokenIssuingTest. Before this change, AuthTokenIssuingTest relied on Kudu treating connections between non-matching addresses in 127.0.0.0/8 subnet as external. With this change, the test forces Kudu client to use an non-loopback IP address for test cases that verify external connections. If an external IP address is not available, test cases that require an external IP will be skipped. This change adds a test case to AuthTokenIssuingTest which verifies that unencripted auth token is passed through unencrypted local connections. Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f --- M src/kudu/integration-tests/security-itest.cc M src/kudu/util/net/socket.cc 2 files changed, 95 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12474/5 -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 5 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12389 ) Change subject: add release notes for 1.9.0 .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/12389/4/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/12389/4/docs/release_notes.adoc@85 PS4, Line 85: n lead to faster scans and faster bootup times -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: comment Gerrit-Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc Gerrit-Change-Number: 12389 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 14 Feb 2019 05:13:55 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1900: add loopback check and test
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12474 to look at the new patch set (#4). Change subject: KUDU-1900: add loopback check and test .. KUDU-1900: add loopback check and test This change modifies Socket::IsLoopbackConnection to check if remote IP is in 127.0.0.0/8 subnet. With this change all connections from 127.0.0.0/8 are treated as local. This change adds force_external_client_ip parameter to AuthTokenIssuingTest. Before this change, AuthTokenIssuingTest relied on Kudu treating connections between non-matching addresses in 127.0.0.0/8 subnet as external. With this change, the test forces Kudu client to use an non-loopback IP address for test cases that verify external connections. If an external IP address is not available, test cases that require an external IP will be skipped. This change adds a test case to AuthTokenIssuingTest which verifies that unencripted auth token is passed through unencrypted local connections. Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f --- M src/kudu/integration-tests/security-itest.cc M src/kudu/util/net/socket.cc 2 files changed, 93 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12474/4 -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 4 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1900: add loopback check and test
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12474 to look at the new patch set (#3). Change subject: KUDU-1900: add loopback check and test .. KUDU-1900: add loopback check and test This change modifies Socket::IsLoopbackConnection to check if remote IP is in 127.0.0.0/8 subnet. With this change all connections from 127.0.0.0/8 are treated as local. This change adds force_external_client_ip parameter to AuthTokenIssuingTest. Before this change, AuthTokenIssuingTest relied on Kudu treating connections between non-matching addresses in 127.0.0.0/8 subnet as external. With this change, the test forces Kudu client to use an non-loopback IP address for test cases that verify external connections. If an external IP address is not available, test cases that require an external IP will be skipped. This change adds a test case to AuthTokenIssuingTest which verifies that unencripted auth token is passed through unencrypted local connections. Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f --- M src/kudu/integration-tests/security-itest.cc M src/kudu/util/net/socket.cc 2 files changed, 92 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12474/3 -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 3 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1900: add loopback check and test
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/12474 ) Change subject: KUDU-1900: add loopback check and test .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@18 PS2, Line 18: #include : #include > Is this approach portable to macOS? This works on macOS as well. I've ran the tests with external IP on my mac. -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 2 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Feb 2019 04:29:01 + Gerrit-HasComments: Yes
[kudu-CR] docs: fix two broken links in the installation doc
Greg Solovyev has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12480 Change subject: docs: fix two broken links in the installation doc .. docs: fix two broken links in the installation doc Change-Id: I249df2ac3a88bda9825419a852a752423a5cb04f --- M docs/installation.adoc 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/12480/1 -- To view, visit http://gerrit.cloudera.org:8080/12480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I249df2ac3a88bda9825419a852a752423a5cb04f Gerrit-Change-Number: 12480 Gerrit-PatchSet: 1 Gerrit-Owner: Greg Solovyev
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12471 ) Change subject: [build-support] KUDU-2699 introduce TIDY build type .. [build-support] KUDU-2699 introduce TIDY build type This changelist introduces a new TIDY build type to facilitate running clang_tidy_gerrit.py from the kudu-tidy-bot Jenkins job by jenkins.kudu.apache.org. The idea is to make sure all auto-generated header files are present before running the clang-tidy tool on the Kudu C++ source files. As Adar noticed, our development process is such that new work goes into master and any commits to release branches are pure backports. There's not much point in running clang-tidy on those backports. So, the proposal is to simply run the kudu-tidy-bot job only on the changes in the master branch. That entails updating the configuration of the kudu-tidy-bot Jenkins job after this change is pushed into the repo. The corresponding command for the config for the kudu-tidy-bot job is: -- \#!/bin/bash export TIDYBOT_PASSWORD= export PATH=/usr/lib/ccache:$PATH ccache -M 50G set -ex export BUILD_TYPE=TIDY build-support/jenkins/build-and-test.sh -- Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Reviewed-on: http://gerrit.cloudera.org:8080/12471 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh 2 files changed, 28 insertions(+), 6 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1900: add loopback check and test
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/12474 ) Change subject: KUDU-1900: add loopback check and test .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@331 PS2, Line 331: INSTANTIATE_TEST_CASE_P(, AuthTokenIssuingTest, ::testing::ValuesIn( > Would it be possible to use ::testing::Combine() to generate this more clea After another look at these, I think I should remove all cases with UNIQUE_LOOPBACK and replace some of them with cases that use an external IP. It looks to me that the only reason for introducing UNIQUE_LOOPBACK into this test was fooling Kudu into thinking that 127.0.0.1 to 127.x.x.x is a non-loopback connection. -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 2 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Feb 2019 01:22:15 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12389 ) Change subject: add release notes for 1.9.0 .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: comment Gerrit-Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc Gerrit-Change-Number: 12389 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 14 Feb 2019 01:15:27 + Gerrit-HasComments: No
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12471 ) Change subject: [build-support] KUDU-2699 introduce TIDY build type .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Feb 2019 01:10:30 + Gerrit-HasComments: No
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12389 ) Change subject: add release notes for 1.9.0 .. Patch Set 3: (18 comments) http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@58 PS3, Line 58: link:TODO[administrative documentation] > This and a few other links are marked as TODO. What's the remaining work he Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@62 PS3, Line 62: An link:https://hub.docker.com/r/apache/kudu[official : repository] has been created for Apache Kudu Docker artifacts > Will this be populated prior to the release? It looks empty right now. That's the idea, yeah. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@64 PS3, Line 64: People wanting to integrate with Kudu > Nit: maybe "Kudu integrators"? Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@64 PS3, Line 64: unit > Nit: drop 'unit'; you can do a Java test at any level. Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@67 PS3, Line 67: This functionality : ships as part of the `kudu-test-utils` Maven module in the `MiniCluster` : class. In order for the `MiniCluster` to automatically find the appropriate : artifacts when starting up, the `kudu-binary` module must be added as a test : time dependency, along with the `kudu-test-utils` module in the project’s : Maven or Gradle build. > Too much implementation detail, I think. Can we link to a more detailed pag Yeah, this will need some updating based on publishing the binaries. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@80 PS3, Line 80: cut by a factor of its replication factor. > I think I wrote this, but can you think of a clearer way to say it? What we I think this is clear enough as is, though I'll add a note that partitions can still be added post-creation, in case less-familiar users are surprised by this note. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@84 PS3, Line 84: order (see KUDU-1400). > For this and other JIRAs, could we insert direct links to the JIRAs themsel Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@85 PS3, Line 85: is > was Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@91 PS3, Line 91: * The Kudu-Spark example can now work against a Kudu cluster with a single : tablet server and accepts a custom replication factor as a parameter. > What makes this noteworthy? Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@93 PS3, Line 93: As a part of this upgrade `spark-avro` : was migrated from the Databricks implementation to the implementation now : included in Apache Spark. This has been done in an API-compatible way. > This seems like an implementation detail; why do Kudu users care about this Done, it doesn't affect the user much so I'm removing this. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@102 PS3, Line 102: is > has been Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@105 PS3, Line 105: * The amount of server-side logging has been greatly reduced for Kudu's : consensus implementation and background processes. > May want to add a tiny bit of color here. Just to talk about how we removed Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@107 PS3, Line 107: will now more obviously depict > now more obviously depicts Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@109 PS3, Line 109: is added > was added Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@112 PS3, Line 112: supporting : very basic predicates > A bit of color to explain the format in which predicates are described? Or Done. Also the details are described in the tool's help blurb. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@121 PS3, Line 121: will now : detect and report > now detects and reports Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@128 PS3, Line 128: The `--cmeta_force_fsync` flag is introduced to fsync Kudu's consensus : metadata more agressively > The new ... flag may be used to fsync Kudu's consensus metadata more aggres Done http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@130 PS3, Line 130: improve its handling of > improve its durability in the face of Done -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-Messag
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Hello Will Berkeley, Alexey Serbin, Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12389 to look at the new patch set (#4). Change subject: add release notes for 1.9.0 .. add release notes for 1.9.0 TODO: clarify binary release Staged version here: https://github.com/andrwng/kudu/blob/branch-1.9.0/docs/release_notes.adoc Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc --- M docs/release_notes.adoc 1 file changed, 105 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/12389/4 -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc Gerrit-Change-Number: 12389 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] log block manager: fix invalid pointer
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12477 ) Change subject: log_block_manager: fix invalid pointer .. log_block_manager: fix invalid pointer We saw a core dump after enabling VLOGing caused by an invalid pointer during OpenBlock(). We seem to be dereferencing a block that has been moved already. Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Reviewed-on: http://gerrit.cloudera.org:8080/12477 Reviewed-by: Adar Dembo Reviewed-by: Hao Hao Tested-by: Kudu Jenkins --- M src/kudu/fs/log_block_manager.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Hao Hao: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12471 to look at the new patch set (#6). Change subject: [build-support] KUDU-2699 introduce TIDY build type .. [build-support] KUDU-2699 introduce TIDY build type This changelist introduces a new TIDY build type to facilitate running clang_tidy_gerrit.py from the kudu-tidy-bot Jenkins job by jenkins.kudu.apache.org. The idea is to make sure all auto-generated header files are present before running the clang-tidy tool on the Kudu C++ source files. As Adar noticed, our development process is such that new work goes into master and any commits to release branches are pure backports. There's not much point in running clang-tidy on those backports. So, the proposal is to simply run the kudu-tidy-bot job only on the changes in the master branch. That entails updating the configuration of the kudu-tidy-bot Jenkins job after this change is pushed into the repo. The corresponding command for the config for the kudu-tidy-bot job is: -- \#!/bin/bash export TIDYBOT_PASSWORD= export PATH=/usr/lib/ccache:$PATH ccache -M 50G set -ex export BUILD_TYPE=TIDY build-support/jenkins/build-and-test.sh -- Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh 2 files changed, 28 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/12471/6 -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12471 ) Change subject: [build-support] KUDU-2699 introduce TIDY build type .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12471/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12471/5//COMMIT_MSG@20 PS5, Line 20: but bot -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Feb 2019 00:19:19 + Gerrit-HasComments: Yes
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12471 to look at the new patch set (#5). Change subject: [build-support] KUDU-2699 introduce TIDY build type .. [build-support] KUDU-2699 introduce TIDY build type This changelist introduces a new TIDY build type to facilitate running clang_tidy_gerrit.py from the kudu-tidy-bot Jenkins job by jenkins.kudu.apache.org. The idea is to make sure all auto-generated header files are present before running the clang-tidy tool on the Kudu C++ source files. As Adar noticed, our development process is such that new work goes into master and any commits to release branches are pure backports. There's not much point in running clang-tidy on those backports. So, the proposal is to simply run the kudu-tidy-but job only on the changes in the master branch. That entails updating the configuration of the kudu-tidy-but Jenkins job after this change is pushed into the repo. The corresponding command for the config for the kudu-tidy-but job is: -- \#!/bin/bash export TIDYBOT_PASSWORD= export PATH=/usr/lib/ccache:$PATH ccache -M 50G set -ex export BUILD_TYPE=TIDY build-support/jenkins/build-and-test.sh -- Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh 2 files changed, 28 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/12471/5 -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] log block manager: fix invalid pointer
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12477 ) Change subject: log_block_manager: fix invalid pointer .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 23:50:20 + Gerrit-HasComments: No
[kudu-CR] log block manager: fix invalid pointer
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12477 ) Change subject: log_block_manager: fix invalid pointer .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc@2063 PS1, Line 2063: VLOG(3) << "Opened block " << block_id > You don't need the ToString() call; there's an operator<< overload for Bloc Done http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc@2065 PS1, Line 2065: block->reset(new internal::LogReadableBlock(std::move(lb))); > Hmm, I bet someone reordered this w.r.t. the VLOG. That's an alternative so This seems simpler, given the std::move(lb) -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 23:33:58 + Gerrit-HasComments: Yes
[kudu-CR] log block manager: fix invalid pointer
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12477 ) Change subject: log_block_manager: fix invalid pointer .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 23:42:59 + Gerrit-HasComments: No
[kudu-CR] log block manager: fix invalid pointer
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12477 to look at the new patch set (#2). Change subject: log_block_manager: fix invalid pointer .. log_block_manager: fix invalid pointer We saw a core dump after enabling VLOGing caused by an invalid pointer during OpenBlock(). We seem to be dereferencing a block that has been moved already. Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a --- M src/kudu/fs/log_block_manager.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/12477/2 -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] log block manager: fix invalid pointer
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12477 ) Change subject: log_block_manager: fix invalid pointer .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc@2063 PS1, Line 2063: VLOG(3) << "Opened block " << block_id.ToString() You don't need the ToString() call; there's an operator<< overload for BlockId. http://gerrit.cloudera.org:8080/#/c/12477/1/src/kudu/fs/log_block_manager.cc@2065 PS1, Line 2065: block->reset(new internal::LogReadableBlock(std::move(lb))); Hmm, I bet someone reordered this w.r.t. the VLOG. That's an alternative solution if you prefer. -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 23:21:37 + Gerrit-HasComments: Yes
[kudu-CR] log block manager: fix invalid pointer
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12477 Change subject: log_block_manager: fix invalid pointer .. log_block_manager: fix invalid pointer We saw a core dump after enabling VLOGing caused by an invalid pointer during OpenBlock(). We seem to be dereferencing a block that has been moved already. Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a --- M src/kudu/fs/log_block_manager.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/12477/1 -- To view, visit http://gerrit.cloudera.org:8080/12477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic6567f43a30c74abc4fcf677671737035e845c1a Gerrit-Change-Number: 12477 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-1900: add loopback check and test
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/12474 )
Change subject: KUDU-1900: add loopback check and test
..
Patch Set 2:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@18
PS2, Line 18: #include
: #include
Is this approach portable to macOS?
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@331
PS2, Line 331: INSTANTIATE_TEST_CASE_P(, AuthTokenIssuingTest,
::testing::ValuesIn(
Would it be possible to use ::testing::Combine() to generate this more clearly?
Could we structure the generators such that we get the right combinations (i.e.
nearly the Cartesian product)?
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@376
PS2, Line 376: // an extnernal IP, so that the connection is not considered
to be local
external
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@378
PS2, Line 378: struct ifaddrs *ifap = nullptr;
The initialization isn't necessary; getifaddrs() is going to overwrite 'ifap'.
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@380
PS2, Line 380: if (getifaddrs(&ifap) > -1) {
Would be cleaner if this could early-out rather than introduce a new nested
scope. Maybe decompose all of this into a helper method?
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@382
PS2, Line 382: if (ifa->ifa_addr == nullptr || ifa->ifa_netmask ==
nullptr || ifa->ifa_addr->sa_family != AF_INET) continue;
Wrap, too long.
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@383
PS2, Line 383: struct sockaddr_in *addr_in = (struct sockaddr_in
*)ifa->ifa_addr;
Use C++-style casts here (static_cast or reinterpret_cast).
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@388
PS2, Line 388: FLAGS_local_ip_for_outbound_sockets.assign(s,
INET_ADDRSTRLEN);
Probably clearer as:
FLAGS_foo = string(s, arraysize(s));
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/integration-tests/security-itest.cc@392
PS2, Line 392: freeifaddrs(ifap);
Use RAII principles: set up a SCOPED_CLEANUP that calls freeifaddrs() when it
goes out of scope, declared just after getifaddrs() is known to have succeeded.
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/util/net/socket.cc
File src/kudu/util/net/socket.cc:
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/util/net/socket.cc@302
PS2, Line 302: // Check if remote address is in 127.0.0.0/8 subnet
Nit: terminate with a period.
Applies to the new comments in the test too.
http://gerrit.cloudera.org:8080/#/c/12474/2/src/kudu/util/net/socket.cc@303
PS2, Line 303: if(
Nit: if (remote
--
To view, visit http://gerrit.cloudera.org:8080/12474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f
Gerrit-Change-Number: 12474
Gerrit-PatchSet: 2
Gerrit-Owner: Greg Solovyev
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 Feb 2019 22:53:15 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1900: add loopback check and test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12474 to look at the new patch set (#2). Change subject: KUDU-1900: add loopback check and test .. KUDU-1900: add loopback check and test This change modifies Socket::IsLoopbackConnection to check if remote IP is in 127.0.0.0/8 subnet. With this change all connections from 127.0.0.0/8 are treated as local. This change adds force_external_client_ip parameter to AuthTokenIssuingTest. Before this change, AuthTokenIssuingTest relied on Kudu treating connections between non-matching addresses in 127.0.0.0/8 subnet as external. With this change, the test forces Kudu client to use an non-loopback IP address for test cases that verify external connections. If an external IP address is not available, test cases that require an external IP will be skipped. This change adds a test case to AuthTokenIssuingTest which verifies that unencripted auth token is passed through unencrypted local connections. Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f --- M src/kudu/integration-tests/security-itest.cc M src/kudu/util/net/socket.cc 2 files changed, 51 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12474/2 -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 2 Gerrit-Owner: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1900: add loopback check and test
Greg Solovyev has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12474 Change subject: KUDU-1900: add loopback check and test .. KUDU-1900: add loopback check and test This change modifies Socket::IsLoopbackConnection to check if remote IP is in 127.0.0.0/8 subnet. With this change all connections from 127.0.0.0/8 are treated as local. This change adds force_external_client_ip parameter to AuthTokenIssuingTest. Before this change, AuthTokenIssuingTest relied on Kudu treating connections between non-matching addresses in 127.0.0.0/8 subnet as external. With this change, the test forces Kudu client to use an non-loopback IP address for test cases that verify external connections. If an external IP address is not available, test cases that require an external IP will be skipped. This change adds a test case to AuthTokenIssuingTest which verifies that unencripted auth token is passed through unencrypted local connections. Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f --- M src/kudu/integration-tests/security-itest.cc M src/kudu/util/net/socket.cc 2 files changed, 51 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12474/1 -- To view, visit http://gerrit.cloudera.org:8080/12474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f Gerrit-Change-Number: 12474 Gerrit-PatchSet: 1 Gerrit-Owner: Greg Solovyev
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12471 ) Change subject: [build-support] KUDU-2699 introduce TIDY build type .. Patch Set 1: Also bear in mind that kudu-tidy-bot is currently configured to run on Gerrit changes to _any_ branch. So if this change is merged, we'll need to deal with older branches somehow. Either by backporting this change, or by reconfiguring kudu-tidy-bot to only run against master. -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 21:54:53 + Gerrit-HasComments: No
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12471 ) Change subject: [build-support] KUDU-2699 introduce TIDY build type .. Patch Set 1: Can you also post your proposed kudu-tidy-bot job config so it can be reviewed alongside the build-and-test.sh changes? As a comment in this gerrit is fine; no need to get fancy. -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 13 Feb 2019 21:49:51 + Gerrit-HasComments: No
[kudu-CR] [build-support] KUDU-2699 introduce TIDY build type
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12471 Change subject: [build-support] KUDU-2699 introduce TIDY build type .. [build-support] KUDU-2699 introduce TIDY build type This changelist introduces a new TIDY build type to facilitate running clang_tidy_gerrit.py from the kudu-tidy-bot Jenkins job by jenkins.kudu.apache.org. The idea is to make sure all auto-generated header files are present before running the clang-tidy tool on the Kudu C++ source files. Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh 2 files changed, 29 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/12471/1 -- To view, visit http://gerrit.cloudera.org:8080/12471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9b364620c9fb92202cacf0286df292dd4cc9952b Gerrit-Change-Number: 12471 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12389 ) Change subject: add release notes for 1.9.0 .. Patch Set 3: (18 comments) http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@58 PS3, Line 58: link:TODO[administrative documentation] This and a few other links are marked as TODO. What's the remaining work here? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@62 PS3, Line 62: An link:https://hub.docker.com/r/apache/kudu[official : repository] has been created for Apache Kudu Docker artifacts Will this be populated prior to the release? It looks empty right now. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@64 PS3, Line 64: People wanting to integrate with Kudu Nit: maybe "Kudu integrators"? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@64 PS3, Line 64: unit Nit: drop 'unit'; you can do a Java test at any level. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@67 PS3, Line 67: This functionality : ships as part of the `kudu-test-utils` Maven module in the `MiniCluster` : class. In order for the `MiniCluster` to automatically find the appropriate : artifacts when starting up, the `kudu-binary` module must be added as a test : time dependency, along with the `kudu-test-utils` module in the project’s : Maven or Gradle build. Too much implementation detail, I think. Can we link to a more detailed page and/or example instead? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@80 PS3, Line 80: cut by a factor of its replication factor. I think I wrote this, but can you think of a clearer way to say it? What we're trying to say is that if you had previously overridden that gflag, your effective max size for a new table of RF=3 is 1/3 the value of that gflag. "Cut by a factor of ..." isn't great because "factor of" suggests an additional scalar applied to the RF. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@84 PS3, Line 84: order (see KUDU-1400). For this and other JIRAs, could we insert direct links to the JIRAs themselves? That's what we did for prior release notes. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@85 PS3, Line 85: is was http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@91 PS3, Line 91: * The Kudu-Spark example can now work against a Kudu cluster with a single : tablet server and accepts a custom replication factor as a parameter. What makes this noteworthy? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@93 PS3, Line 93: As a part of this upgrade `spark-avro` : was migrated from the Databricks implementation to the implementation now : included in Apache Spark. This has been done in an API-compatible way. This seems like an implementation detail; why do Kudu users care about this bit? What does it mean for them? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@102 PS3, Line 102: is has been http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@105 PS3, Line 105: * The amount of server-side logging has been greatly reduced for Kudu's : consensus implementation and background processes. May want to add a tiny bit of color here. Just to talk about how we removed logging that didn't seem useful and was unnecessarily verbose. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@107 PS3, Line 107: will now more obviously depict now more obviously depicts http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@109 PS3, Line 109: is added was added Below too. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@112 PS3, Line 112: supporting : very basic predicates A bit of color to explain the format in which predicates are described? Or too much detail? http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@121 PS3, Line 121: will now : detect and report now detects and reports http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@128 PS3, Line 128: The `--cmeta_force_fsync` flag is introduced to fsync Kudu's consensus : metadata more agressively The new ... flag may be used to fsync Kudu's consensus metadata more aggressively. http://gerrit.cloudera.org:8080/#/c/12389/3/docs/release_notes.adoc@130 PS3, Line 130: improve its handling of improve its durability in the face of -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: com
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12122 ) Change subject: KUDU-2543 pt 2: pass around default authz tokens .. KUDU-2543 pt 2: pass around default authz tokens Adds authz token generation to the master's GetTableSchema endpoint, with which clients can authorize themselves for specific tables. A client will cache these tokens and use them appropriately for RPCs that need them (e.g. Writes and Scans), reacquiring them when receiving word that they are expired. This is tested in the following ways: - unit tests for the new client-side cache for authz tokens - parameterized the token expiration test for authn and authz tokens to have varying token expirations, testing when authn tokens expire but not authz tokens, and vice versa - added various tests to ensure the client behaves correctly in various non-happy cases Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Reviewed-on: http://gerrit.cloudera.org:8080/12122 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Hao Hao --- M src/kudu/client/CMakeLists.txt A src/kudu/client/authz_token_cache.cc A src/kudu/client/authz_token_cache.h M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.h M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/auth_token_expire-itest.cc A src/kudu/integration-tests/authz_token-itest.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc.h M src/kudu/util/test_util.h 20 files changed, 1,347 insertions(+), 95 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Gerrit-Change-Number: 12122 Gerrit-PatchSet: 13 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12410 ) Change subject: [spark] Factor out the Spark/Kudu row converter .. [spark] Factor out the Spark/Kudu row converter This patch factors out the Spark/Kudu row converter so that it can be used more generically in the Spark integration and in other Spark tools. Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Reviewed-on: http://gerrit.cloudera.org:8080/12410 Reviewed-by: Adar Dembo Tested-by: Grant Henke Reviewed-by: Hao Hao --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala 2 files changed, 121 insertions(+), 46 deletions(-) Approvals: Adar Dembo: Looks good to me, but someone else must approve Grant Henke: Verified Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12389 ) Change subject: add release notes for 1.9.0 .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12389/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/12389/2/docs/release_notes.adoc@78 PS2, Line 78: rather than > Yeah, see Todd's comment: Makes sense, thanks! -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: comment Gerrit-Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc Gerrit-Change-Number: 12389 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 19:16:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12122 ) Change subject: KUDU-2543 pt 2: pass around default authz tokens .. Patch Set 12: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Gerrit-Change-Number: 12122 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 13 Feb 2019 19:14:29 + Gerrit-HasComments: No
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12410 ) Change subject: [spark] Factor out the Spark/Kudu row converter .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 19:11:25 + Gerrit-HasComments: No
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Grant Henke has removed a vote on this change. Change subject: [spark] Factor out the Spark/Kudu row converter .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12410 ) Change subject: [spark] Factor out the Spark/Kudu row converter .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 19:05:28 + Gerrit-HasComments: No
[kudu-CR](branch-1.9.x) add release notes for 1.9.0
Hello Will Berkeley, Alexey Serbin, Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12389 to look at the new patch set (#3). Change subject: add release notes for 1.9.0 .. add release notes for 1.9.0 TODO: add links Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc --- M docs/release_notes.adoc 1 file changed, 88 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/12389/3 -- To view, visit http://gerrit.cloudera.org:8080/12389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.9.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I733dfae39c06f15f7f55ae823678caf6ca433bfc Gerrit-Change-Number: 12389 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. [Java] Add setRow to Operation Adds a setRow method to Operation to allow for existing rows to be added to an operation vs being manually built. Change-Id: I7adee20166e90249209e80700db315172669edb5 Reviewed-on: http://gerrit.cloudera.org:8080/12409 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2693. Buffer blocks while flushing rowsets
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12425 ) Change subject: KUDU-2693. Buffer blocks while flushing rowsets .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12425/1/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/12425/1/src/kudu/fs/block_manager-stress-test.cc@317 PS1, Line 317: CHECK_OK(bm_->CreateBlock(CreateBlockOptions().with_tablet_id(test_tablet_name_), &block)); Would also be interesting to choose some blocks at random to buffer. http://gerrit.cloudera.org:8080/#/c/12425/1/src/kudu/fs/block_manager-stress-test.cc@350 PS1, Line 350: CHECK_OK(dirty_block->Finalize()); And maybe randomly select some blocks NOT to finalize. -- To view, visit http://gerrit.cloudera.org:8080/12425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacc662ba812ece8e68b0ef28f4ccdf0b7475fbc0 Gerrit-Change-Number: 12425 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 13 Feb 2019 17:39:29 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12410 ) Change subject: [spark] Factor out the Spark/Kudu row converter .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12410/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/12410/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@362 PS1, Line 362: val kuduSchema = table.getSchema : val rowConverter = new RowConverter(kuduSchema, schema, writeOptions.ignoreNull) > Combine? Done -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:34:41 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12393 ) Change subject: [Java] Add PartialRow and RowResult getObject API .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:28:28 + Gerrit-HasComments: No
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema().equals(table.getSchema()), : "The row's schema must be equal to the table schema"); > Ah, I assumed Schema had an overridden equals() that behaved like its C++ e I would like for it to have an overridden equals, but some if it's internals make that difficult. There is some history on that somewhere. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:36:25 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema() == table.getSchema(), : "The row's schema must be equal by reference to the ta > There is a cost, but it's pretty low given they need to be equal by referen Ah, I assumed Schema had an overridden equals() that behaved like its C++ equivalent. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:35:29 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12410 ) Change subject: [spark] Factor out the Spark/Kudu row converter .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:32:51 + Gerrit-HasComments: No
[kudu-CR] Fix compilation warnings under debian8.9
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12294 ) Change subject: Fix compilation warnings under debian8.9 .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/bshuf_block.cc File src/kudu/cfile/bshuf_block.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/bshuf_block.cc@99 PS1, Line 99: uint32_t mid_key = 0; > Done I don't think this has been addressed yet. http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc File src/kudu/consensus/leader_election.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc@304 PS1, Line 304: CHECK_OK(vote_counter_->GetDecision(&decision)); > Done PS2 still shows a default value of VOTE_DENIED instead of suppression of the warning. http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/cfile_set.cc@320 PS1, Line 320: boost::optional opt_rowid; > all right, I gave up on fixing it Why not add the suppression though? Using this: #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" #pragma GCC diagnostic pop -- To view, visit http://gerrit.cloudera.org:8080/12294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2464ff7938f79d090ebd1676da2108ce7dae4ca5 Gerrit-Change-Number: 12294 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 13 Feb 2019 17:32:09 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Factor out the Spark/Kudu row converter
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12410 to look at the new patch set (#2). Change subject: [spark] Factor out the Spark/Kudu row converter .. [spark] Factor out the Spark/Kudu row converter This patch factors out the Spark/Kudu row converter so that it can be used more generically in the Spark integration and in other Spark tools. Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala A java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala 2 files changed, 121 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/12410/2 -- To view, visit http://gerrit.cloudera.org:8080/12410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89ae6f334134ec47084972c95cfe57a71aa3b68d Gerrit-Change-Number: 12410 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Grant Henke has removed a vote on this change. Change subject: [Java] Add PartialRow and RowResult getObject API .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12393 ) Change subject: [Java] Add PartialRow and RowResult getObject API .. [Java] Add PartialRow and RowResult getObject API This patch adds getObject methods to PartialRow and RowResult to prevent people from repeating this bit of code over and over. Also replaces 2 places where this pattern is used. Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Reviewed-on: http://gerrit.cloudera.org:8080/12393 Reviewed-by: Adar Dembo Tested-by: Grant Henke --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 6 files changed, 188 insertions(+), 48 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Grant Henke: Verified -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12409/1//COMMIT_MSG Commit Message: PS1: > Not really understanding the motivation. You provided some sample code near This is useful for integration use cases where you translate from some third-party row class to Kudu's partial row and then convert it to an operation to send to kudu. Today you need to know the operation before you convert which is more tightly coupled than needed, or you need to copy the partial row over to the ops partial row which is expensive. The next 2 patches in this series are a good example of usage. Additionally I have a repartitioning patch coming up that will use this. http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@188 PS1, Line 188: tables > table's Done http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema().equals(table.getSchema()), : "The row's schema must be equal to the table schema"); > Won't this be expensive when working with many operations? There is a cost, but it's pretty low given they need to be equal by reference, not by value. I will change to using == incase the equals implementation ever changes. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:20:07 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12409 to look at the new patch set (#2). Change subject: [Java] Add setRow to Operation .. [Java] Add setRow to Operation Adds a setRow method to Operation to allow for existing rows to be added to an operation vs being manually built. Change-Id: I7adee20166e90249209e80700db315172669edb5 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/12409/2 -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2674: [java] Add a Java KuduPartitioner API
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12275 ) Change subject: KUDU-2674: [java] Add a Java KuduPartitioner API .. KUDU-2674: [java] Add a Java KuduPartitioner API This patch is a Java port of the c++ KuduPartitioner API introduced in KUDU-1713 (https://gerrit.cloudera.org/#/c/5775/). The API allows a client to determine which partition a row falls into without actually writing that row. This would allow Spark and other Java integrations to repartition and pre-sort the data before writing to Kudu. This patch also fixes a bug where calls to AsyncKuduClient.locateTable could take much longer than the specified timeout. The timeout was not propogated to subsequent locateTablet call and each locateTablet used the default admin operation timeout as a result. Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478 Reviewed-on: http://gerrit.cloudera.org:8080/12275 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java M src/kudu/client/client-test.cc M src/kudu/client/client.h 6 files changed, 430 insertions(+), 8 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478 Gerrit-Change-Number: 12275 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2674: [java] Add a Java KuduPartitioner API
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12275 ) Change subject: KUDU-2674: [java] Add a Java KuduPartitioner API .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478 Gerrit-Change-Number: 12275 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 16:52:58 + Gerrit-HasComments: No
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12393 ) Change subject: [Java] Add PartialRow and RowResult getObject API .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 16:54:32 + Gerrit-HasComments: No
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12393 to look at the new patch set (#3). Change subject: [Java] Add PartialRow and RowResult getObject API .. [Java] Add PartialRow and RowResult getObject API This patch adds getObject methods to PartialRow and RowResult to prevent people from repeating this bit of code over and over. Also replaces 2 places where this pattern is used. Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 6 files changed, 188 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/12393/3 -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add PartialRow and RowResult getObject API
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12393 ) Change subject: [Java] Add PartialRow and RowResult getObject API .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/12393/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12393/2//COMMIT_MSG@10 PS2, Line 10: repeting > repeating Done http://gerrit.cloudera.org:8080/#/c/12393/2//COMMIT_MSG@10 PS2, Line 10: Rowresult > RowResult Done http://gerrit.cloudera.org:8080/#/c/12393/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java File java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java: http://gerrit.cloudera.org:8080/#/c/12393/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@533 PS2, Line 533: columns > Nit: column's Done http://gerrit.cloudera.org:8080/#/c/12393/2/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@591 PS2, Line 591: case BINARY: return getBinaryCopy(columnIndex); > What's the rationale for getBinaryCopy() over getBinary() in this case? The intent was to be as primitive as possible and safe. This should allow most integrations to use this method. For example Spark handles byte[] but not ByteBuffer. -- To view, visit http://gerrit.cloudera.org:8080/12393 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7904a3478896c84fb998d1fddcc2a68c998d4a91 Gerrit-Change-Number: 12393 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 16:43:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2674: [java] Add a Java KuduPartitioner API
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/12275 )
Change subject: KUDU-2674: [java] Add a Java KuduPartitioner API
..
Patch Set 6:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1825
PS6, Line 1825: propogte
> propagate
Done
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2163
PS6, Line 2163:* @param table the table
> Update this list.
Done
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2212
PS6, Line 2212: if (lookupType == LookupType.POINT ||
entry.getUpperBoundPartitionKey().length == 0) {
> Too long; wrap?
Done
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java:
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java@178
PS6, Line 178: // Ensure the table information can't be found to force a
timeout.
: harness.killAllMasterServers();
> Hmm, surprised the partitioner use below doesn't hit the table locations ca
client.createTable doesn't populate the cache with all the partition
information. I will add a test that proves the cache is being used.
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java@182
PS6, Line 182: long now = System.currentTimeMillis();
> Nit: probably more useful as 'start'.
Done
http://gerrit.cloudera.org:8080/#/c/12275/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java@192
PS6, Line 192: assertTrue(elapsed <= timeoutMs);
> Why would this hold? Seems potentially flaky. How about < timeoutMs * 2 or
I haven't seen flakiness. I can add a small amount of time but I don't want to
double the timeout since that isn't representative of validating the timeout.
--
To view, visit http://gerrit.cloudera.org:8080/12275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478
Gerrit-Change-Number: 12275
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Wed, 13 Feb 2019 16:17:52 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2674: [java] Add a Java KuduPartitioner API
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12275 to look at the new patch set (#7). Change subject: KUDU-2674: [java] Add a Java KuduPartitioner API .. KUDU-2674: [java] Add a Java KuduPartitioner API This patch is a Java port of the c++ KuduPartitioner API introduced in KUDU-1713 (https://gerrit.cloudera.org/#/c/5775/). The API allows a client to determine which partition a row falls into without actually writing that row. This would allow Spark and other Java integrations to repartition and pre-sort the data before writing to Kudu. This patch also fixes a bug where calls to AsyncKuduClient.locateTable could take much longer than the specified timeout. The timeout was not propogated to subsequent locateTablet call and each locateTablet used the default admin operation timeout as a result. Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java A java/kudu-client/src/main/java/org/apache/kudu/client/KuduPartitioner.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPartitioner.java M src/kudu/client/client-test.cc M src/kudu/client/client.h 6 files changed, 430 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/12275/7 -- To view, visit http://gerrit.cloudera.org:8080/12275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7a2d47aab5318c0b6d29a8cb2b073c05bc1b6478 Gerrit-Change-Number: 12275 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Fix compilation warnings under debian8.9
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12294 to look at the new patch set (#2). Change subject: Fix compilation warnings under debian8.9 .. Fix compilation warnings under debian8.9 It seems there are some compilation warnings under debian8.9, with gcc 4.9.2/5.5.0, and the warning info is formatted as follows: "warning: ‘xxx’ may be used uninitialized in this function" Change-Id: I2464ff7938f79d090ebd1676da2108ce7dae4ca5 --- M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/rle_block.h M src/kudu/common/row_operations.cc M src/kudu/consensus/leader_election.cc M src/kudu/consensus/log-test.cc M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/hms/hms_client-test.cc M src/kudu/integration-tests/log_verifier.cc M src/kudu/rpc/server_negotiation.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_perf.cc M src/kudu/tserver/mini_tablet_server-test.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/net/sockaddr.cc M src/kudu/util/pb_util.cc M src/kudu/util/status.cc 20 files changed, 32 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/12294/2 -- To view, visit http://gerrit.cloudera.org:8080/12294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2464ff7938f79d090ebd1676da2108ce7dae4ca5 Gerrit-Change-Number: 12294 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] Fix compilation warnings under debian8.9
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12294 ) Change subject: Fix compilation warnings under debian8.9 .. Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/rle_block.h File src/kudu/cfile/rle_block.h: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/cfile/rle_block.h@380 PS1, Line 380: CppType cur_elem = 0; > Ideally I'd say we should CHECK() and crash if cur_elem wasn't set, but tha Done. In addition, the reason why don't put the 'cur_elem' out of the loop is that the worry your mentioned previously. http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: PS1: > In this case I think initialization to nullptr is OK, because it'll at leas Done http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc File src/kudu/consensus/leader_election.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/consensus/leader_election.cc@304 PS1, Line 304: CHECK_OK(vote_counter_->GetDecision(&decision)); > This is more concerning; if we were somehow wrong, elections may fail. That Done http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/fs/log_block_manager-test-util.cc File src/kudu/fs/log_block_manager-test-util.cc: PS1: > This is test code so I don't really care; a nullptr is fine I guess. Done http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/hms/hms_client-test.cc File src/kudu/hms/hms_client-test.cc: PS1: > That's totally bizarre; there are tests for 'protection' in the same functi i think it will be much more acceptable through 'boost::make_optional' to initialize the variable(boost::none). http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/integration-tests/log_verifier.cc File src/kudu/integration-tests/log_verifier.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/integration-tests/log_verifier.cc@151 PS1, Line 151: int64_t expected_term = kNotOnThisServer; > 1.the original warning is: it seems boost::make_optional also works :) http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/cfile_set.cc@320 PS1, Line 320: boost::optional opt_rowid(0); > This is a hot path; I'd prefer to suppress the warning. Especially because all right, I gave up on fixing it http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/tablet-test-util.h File src/kudu/tablet/tablet-test-util.h: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/tablet/tablet-test-util.h@661 PS1, Line 661: int64_t prev_row_idx = -1; > boost::optional here is semantically useful; it indicates that something ma yep, it works:) http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/util/status.cc File src/kudu/util/status.cc: http://gerrit.cloudera.org:8080/#/c/12294/1/src/kudu/util/status.cc@49 PS1, Line 49: const char* type = nullptr; > Can we add a default case with a LOG(FATAL) instead? Done -- To view, visit http://gerrit.cloudera.org:8080/12294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2464ff7938f79d090ebd1676da2108ce7dae4ca5 Gerrit-Change-Number: 12294 Gerrit-PatchSet: 1 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 13 Feb 2019 12:05:45 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2514 Support extra config for table.
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12468 to look at the new patch set (#2). Change subject: KUDU-2514 Support extra config for table. .. KUDU-2514 Support extra config for table. This patch implements the extra-config for table. Now, we can specify the history max age second of the table by this feature. Change-Id: I0514507dca95602a97e954d1db464b907e073aae --- M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/common/common.proto M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc M src/kudu/master/sys_catalog.cc M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/transactions/alter_schema_transaction.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto M www/table.mustache 33 files changed, 299 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/12468/2 -- To view, visit http://gerrit.cloudera.org:8080/12468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0514507dca95602a97e954d1db464b907e073aae Gerrit-Change-Number: 12468 Gerrit-PatchSet: 2 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2514 Support extra config for table.
Yao Xu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12468 Change subject: KUDU-2514 Support extra config for table. .. KUDU-2514 Support extra config for table. This patch implements the extra-config for table. Now, we can specify the history max age second of the table by this feature. Change-Id: I0514507dca95602a97e954d1db464b907e073aae --- M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/common/common.proto M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc M src/kudu/master/sys_catalog.cc M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet_bootstrap-test.cc M src/kudu/tablet/tablet_metadata-test.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/transactions/alter_schema_transaction.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc M src/kudu/tserver/mini_tablet_server.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto M www/table.mustache 33 files changed, 283 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/12468/1 -- To view, visit http://gerrit.cloudera.org:8080/12468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0514507dca95602a97e954d1db464b907e073aae Gerrit-Change-Number: 12468 Gerrit-PatchSet: 1 Gerrit-Owner: Yao Xu
[kudu-CR] [sentry] Integrate AuthzProvider into CatalogManager
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11797 )
Change subject: [sentry] Integrate AuthzProvider into CatalogManager
..
Patch Set 7:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/11797/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11797/7//COMMIT_MSG@20
PS7, Line 20: exposedp
exposed
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/client/client-test.cc@293
PS7, Line 293: /* rpc = */
Nit: convention is more like this:
/*rpc=*/nullptr
See https://gerrit.cloudera.org/c/12415/3/src/kudu/common/schema.cc#196 for
more context.
Applies elsewhere in your patch too.
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/CMakeLists.txt@91
PS7, Line 91: ADD_KUDU_TEST(master_sentry-itest RUN_SERIAL true PROCESSORS 4)
: ADD_KUDU_TEST(master_sentry_advance-itest RUN_SERIAL true
PROCESSORS 4)
Do these need to be sharded, given the parameterized tests in each and the time
it takes to start a cluster with Sentry and HMS? How long does each take to
run, in isolation?
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/hms_itest-base.h
File src/kudu/integration-tests/hms_itest-base.h:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/hms_itest-base.h@52
PS7, Line 52: using client::KuduColumnSchema;
: using client::KuduSchema;
: using client::KuduSchemaBuilder;
: using client::KuduTable;
: using client::KuduTableCreator;
: using client::sp::shared_ptr;
: using hms::HmsClient;
: using std::string;
: using std::unique_ptr;
: using strings::Substitute;
> I'm not sure that 'using ...' in a header file is a good idea.
Agreed with Alexey; I don't think this is a pattern we should promote. Almost
all the current usages of this are from old code.
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry-itest.cc
File src/kudu/integration-tests/master_sentry-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry-itest.cc@31
PS7, Line 31: // IWYU pragma: no_include
"kudu/integration-tests/hms_itest-base.h"
Again, what happened here?
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry-itest.cc@63
PS7, Line 63: ASSERT_EQ(set({ Substitute("$0.$1", kDatabaseName,
kTableName),
Nit: unordered_set is more representative of the semantics you care about (i.e.
deduplication but unsorted).
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry-itest.cc@98
PS7, Line 98: // Authorize create table.
These comments don't add any useful information.
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc
File src/kudu/integration-tests/master_sentry_advance-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc@31
PS7, Line 31: // IWYU pragma: no_include
"kudu/integration-tests/hms_itest-base.h"
What happened here? Why shouldn't we include this?
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc@37
PS7, Line 37: advanced
> What is "advanced" referring to?
Also not really seeing the distinction; why can't these tests just live in
master_sentry-itest?
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc@91
PS7, Line 91: altering
But we're not necessarily altering, right?
Below too.
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc@125
PS7, Line 125: // Authz funcs for delete table .
Nit: misplaced
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/master_sentry_advance-itest.cc@131
PS7, Line 131: // Authz funcs for alter table.
On second thought, these comments aren't adding anything useful.
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/registration-test.cc
File src/kudu/integration-tests/registration-test.cc:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/registration-test.cc@197
PS7, Line 197: s = catalog->GetTabletLocations(tablet_id,
master::VOTER_REPLICA, &loc, nullptr);
need /*rpc=*/ here
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/sentry_itest-base.h
File src/kudu/integration-tests/sentry_itest-base.h:
http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/integration-tests/sentry_itest-base.h@75
PS7, Line 75: class SentryTestBase : public
