[kudu-CR] KUDU-2095 - Add scanner `keepAlive` RPC call to Java API
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/7749 ) Change subject: KUDU-2095 - Add scanner `keepAlive` RPC call to Java API .. Patch Set 9: (5 comments) http://gerrit.cloudera.org:8080/#/c/7749/9/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/7749/9/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1006 PS9, Line 1006: nit: double space http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1007 PS9, Line 1007: defaultAdminOperationTimeoutMs this seems like an odd choice of timeout. I think this defaults to be pretty long, maybe a short one is better? http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1008 PS9, Line 1008: final ReplicaSelection replicaSelection = scanner.getReplicaSelection(); : final ServerInfo info = : tablet.getReplicaSelectedServerInfo(replicaSelection); is this guaranteed to be the same replica that the scanner is already open on? it doesn't seem so, though I'm not super familiar with this code. Given that replica selection might be somehow randomized this might get fired to some different server? http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java: http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java@75 PS9, Line 75: SetFaultTolerant this is the C++ API name http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/7749/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@203 PS9, Line 203: 50% of the scanner ttl. I dont see anywhere setting a short TTL on this test, am I missing something? -- To view, visit http://gerrit.cloudera.org:8080/7749 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba647e7b5ed4ee8d223d387b6656d3fb02c41713 Gerrit-Change-Number: 7749 Gerrit-PatchSet: 9 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Tony Foerster Gerrit-Comment-Date: Tue, 31 Jul 2018 03:00:54 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Atomicize OpId and Timestamp assignment
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11070 to look at the new patch set (#6). Change subject: WIP: Atomicize OpId and Timestamp assignment .. WIP: Atomicize OpId and Timestamp assignment This is a simpler version of a previous patch but without big changes to the queue or consensus. Change-Id: I0d369423dd5c96b653b424c29744507edd874357 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/pending_rounds.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tserver/tablet_server-test.cc 7 files changed, 75 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/11070/6 -- To view, visit http://gerrit.cloudera.org:8080/11070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357 Gerrit-Change-Number: 11070 Gerrit-PatchSet: 6 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10 PS2, Line 10: A new flag allows the database to : be configured, and defaults to the 'default' HMS database. > Should we be more coy about it, to avoid inflating user expectations vis a I don't think being coy will help, but I've added a sentence about being useful primarily for the HMS integration. I've also added an option to restore the previous behavior by setting the flag to the empty string. -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:28:57 + Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11084 to look at the new patch set (#4). Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. perf loadgen: add --auto_database flag and change auto table name generation This commit changes the 'perf loadgen' tool to create HMS-compliant tables when an auto table is created. A new flag allows the database to be configured, and defaults to the 'default' HMS database. Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_perf.cc 2 files changed, 27 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/4 -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms-tool: lookup master addresses config from master
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11083 ) Change subject: hms-tool: lookup master addresses config from master .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG@14 PS3, Line 14: then metadata will be flagged as stale : and rewritten unnecessarily. > OK, what metadata specifically? Can you provide an example? Sure, Kudu writes the master addresses config into the table entry in the HMS under the 'kudu.master_addresses' key. The 'hms check' tool validates that the master address passed on the command line matches that property on all Kudu tables in the HMS. If any do not, the 'hms fix' tool will overwrite the property with the (assumed correct) value provided on the command line. If that value is wrong (e.g. the admin used 'localhost' because they knew they were on the master machine), then the HMS metadata will no longer be correct. The 'check' tool would have also unnecessarily flagged that table as having stale metadata. This commit changes the behavior so that the master addresses provided on the command line are not assumed correct, instead we assume that master's version of the master addresses config is correct. -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:17:48 + Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10 PS2, Line 10: A new flag allows the database to : be configured, and defaults to the 'default' HMS database. > In a few cases I've modified tests to use an HMS-compatible name even thoug Should we be more coy about it, to avoid inflating user expectations vis a vis database support when HMS integration is not being used? For example, we could just advertise the flag as "prefix to include before a dot and the loadgen table name" rather than "database". -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:14:30 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: lookup master addresses config from master
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11083 ) Change subject: hms-tool: lookup master addresses config from master .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG@14 PS3, Line 14: then metadata will be flagged as stale : and rewritten unnecessarily. > Yep, sorry this is referring to metadata stored in a Kudu table entry in th OK, what metadata specifically? Can you provide an example? Also, can you elaborate on the downside of an unnecessary metadata rewrite? Why is it worth this new private API? -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:12:59 + Gerrit-HasComments: Yes
[kudu-CR] hms precheck tool
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
..
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
: case-insensitive identifier rules
> Are there any other funny cases like already existing Kuud tables named lik
Not sure I fully understand your question. It's definitely possible to have a
table named 'default.MegaTable' in a cluster and then go through the workflow
for enabling the HMS integration. In that case the precheck tool would not
flag the table (unless a second table conflicted with it by case such as
'default.MEGATABLE'). The 'hms fix' tool will automatically create an HMS
table for it when run.
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2605
PS7, Line 2605: for (const string& table_name : {
: "a.b",
: "foo.bar",
: "FOO.bar",
> I don't think this is relevant if using CreateKuduTable()
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2626
PS7, Line 2626: ASSERT_STR_CONTAINS(o
> Maybe, it's worth asserting on specific type of error (RunTimeError, etc.)?
That depends on the internals of Subprocess::Call, which isn't really relevant
here, and doesn't appear to be documented (not sure if that's intentional or
not).
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2642
PS7, Line 2642: NO_FATALS(RunAction
> wrap into NO_FATALS()
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2643
PS7, Line 2643:
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2646
PS7, Line 2646: cluster_->EnableMet
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2657
PS7, Line 2657:
: "foo.bar",
: "foo.bar2",
: "foo.bar3",
: "fuzz",
: }), tables);
: }
:
: // This test is parameterized on the serialization mode and
Kerberos.
: cla
> nit: the expected value comes first -- that way it's easier to read the err
Done
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 9
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:10:14 +
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10 PS2, Line 10: A new flag allows the database to : be configured, and defaults to the 'default' HMS database. > This sort-of kind-of exposes the idea of a database beyond just the confine In a few cases I've modified tests to use an HMS-compatible name even though the test only runs with HMS integration enabled via a parameter. This is the first and only non-test code to my knowledge. That follows pretty directly from this being the only non-test code which creates tables. If we wanted to be really fancy we could auto-detect whether the HMS is enabled through the GetFlags API and do something special, but in my opinion it's better to change the behavior now and make it consistent going forward. -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:09:43 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: lookup master addresses config from master
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11083 ) Change subject: hms-tool: lookup master addresses config from master .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG@14 PS3, Line 14: then metadata will be flagged as stale : and rewritten unnecessarily. > For my own edification, what metadata is being referred to here? Kudu metad Yep, sorry this is referring to metadata stored in a Kudu table entry in the HMS. Will update message. http://gerrit.cloudera.org:8080/#/c/11083/3/src/kudu/client/client-internal.h File src/kudu/client/client-internal.h: http://gerrit.cloudera.org:8080/#/c/11083/3/src/kudu/client/client-internal.h@194 PS3, Line 194: const std::vector& master_hostports() const; : > Nit: I'd actually prefer this comment on master_hostports_ because this is Done -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 31 Jul 2018 00:06:35 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: lookup master addresses config from master
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11083 to look at the new patch set (#4). Change subject: hms-tool: lookup master addresses config from master .. hms-tool: lookup master addresses config from master The HMS check and fix tools currently use the master addresses configuration provided on the command line to validate and fix metadata in HMS table entries. This only works correctly if the administrator inputs the master addresses exactly as its configured on the masters. If the hostports are reordered or use a slightly different format which resolves to the same addresses, then metadata will be flagged as stale and rewritten unnecessarily. This commit changes the handling so that the tools use the master addresses returned from the ConnectToCluster mechanism already present in the client, which should match the master addresses config of the master exactly. Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_hms.cc 6 files changed, 55 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/11083/4 -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms precheck tool
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11040 to look at the new patch set (#9). Change subject: hms precheck tool .. hms precheck tool This commit adds a 'kudu hms precheck' tool that is meant to be used before enabling the HMS integration on an existing cluster. The tool checks for tables whose names are not unique when compared with Hive's case-insensitive identifier rules. These conflicting tables would cause the master to fail to startup after enabling the Hive Metastore integration. Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 2 files changed, 143 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/11040/9 -- To view, visit http://gerrit.cloudera.org:8080/11040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e Gerrit-Change-Number: 11040 Gerrit-PatchSet: 9 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11084/2//COMMIT_MSG@10 PS2, Line 10: A new flag allows the database to : be configured, and defaults to the 'default' HMS database. This sort-of kind-of exposes the idea of a database beyond just the confines of an HMS integrated Kudu. Is that something we've already done in past HMS integration patches? -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:54:32 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: lookup master addresses config from master
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11083 ) Change subject: hms-tool: lookup master addresses config from master .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11083/3//COMMIT_MSG@14 PS3, Line 14: then metadata will be flagged as stale : and rewritten unnecessarily. For my own edification, what metadata is being referred to here? Kudu metadata? Something in the HMS? You needn't elaborate in the commit message itself; I'm sure this is just context that I lack since I haven't been reviewing all of the HMS work. http://gerrit.cloudera.org:8080/#/c/11083/3/src/kudu/client/client-internal.h File src/kudu/client/client-internal.h: http://gerrit.cloudera.org:8080/#/c/11083/3/src/kudu/client/client-internal.h@194 PS3, Line 194: // Returns the master RPC host ports as configured on the most recently : // connected to leader master. Nit: I'd actually prefer this comment on master_hostports_ because this is clearly a simple accessor; commenting on how the state itself mutates is more useful. -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:50:29 + Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:50:06 + Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool
..
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11040/7//COMMIT_MSG@11
PS7, Line 11: are not unique when compared with Hive's
: case-insensitive identifier rules
Are there any other funny cases like already existing Kuud tables named like
'default.MegaTable' when HMS would have 'MegaTable' in its default database?
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2605
PS7, Line 2605: KuduSchemaBuilder schema_builder;
:
schema_builder.AddColumn("key")->Type(client::KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
: KuduSchema schema;
: ASSERT_OK(schema_builder.Build());
I don't think this is relevant if using CreateKuduTable()
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2626
PS7, Line 2626: ASSERT_FALSE(s.ok());
Maybe, it's worth asserting on specific type of error (RunTimeError, etc.)?
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2642
PS7, Line 2642: RunActionStdoutNone
wrap into NO_FATALS()
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2643
PS7, Line 2643: RunActionStdoutNone
ditto
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2646
PS7, Line 2646: RunActionStdoutNone
ditto
http://gerrit.cloudera.org:8080/#/c/11040/7/src/kudu/tools/kudu-tool-test.cc@2657
PS7, Line 2657: vector({
: "A.B!",
: "FUZZ",
: "a.b",
: "a.b!",
: "foo.bar",
: "foo.bar2",
: "foo.bar3",
: "fuzz",
: }
nit: the expected value comes first -- that way it's easier to read the error
message (if any).
--
To view, visit http://gerrit.cloudera.org:8080/11040
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e
Gerrit-Change-Number: 11040
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:45:01 +
Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504 PS1, Line 1504: Host > Well, I'm not sure about standard, but I thought it would be either 4 space Done http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1514 PS1, Line 1514: Host > nit: the same question on the spacing Done -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:31:28 + Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Hello Alexey Serbin, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11084 to look at the new patch set (#2). Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. perf loadgen: add --auto_database flag and change auto table name generation This commit changes the 'perf loadgen' tool to create HMS-compliant tables when an auto table is created. A new flag allows the database to be configured, and defaults to the 'default' HMS database. Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_perf.cc 2 files changed, 22 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/2 -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] Add changing master hostnames workflow
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11058 ) Change subject: [docs] Add changing master hostnames workflow .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0df87d5e294d8b7bf5c7b8f94a63599ffd7ebe03 Gerrit-Change-Number: 11058 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 30 Jul 2018 23:28:11 + Gerrit-HasComments: No
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504 PS1, Line 1504: > It's what my editor did automatically, it's two spaces per open paren. Do Well, I'm not sure about standard, but I thought it would be either 4 spaces (i.e. twice the embedded scope shift) or aligned with the opening corresponding opening parenthesis. But it's just a nit, though. -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:27:16 + Gerrit-HasComments: Yes
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504 PS1, Line 1504: > nit: is this spacing correct and intended? It's what my editor did automatically, it's two spaces per open paren. Do we have a standard for this? -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:24:19 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Atomicize OpId and Timestamp assignment
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11070 to look at the new patch set (#5). Change subject: WIP: Atomicize OpId and Timestamp assignment .. WIP: Atomicize OpId and Timestamp assignment This is a simpler version of a previous patch but without big changes to the queue or consensus. Change-Id: I0d369423dd5c96b653b424c29744507edd874357 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/pending_rounds.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tserver/tablet_server-test.cc 7 files changed, 74 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/11070/5 -- To view, visit http://gerrit.cloudera.org:8080/11070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357 Gerrit-Change-Number: 11070 Gerrit-PatchSet: 5 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11084 ) Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. Patch Set 1: (2 comments) lgtm, just a couple of formatting nits http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1504 PS1, Line 1504: nit: is this spacing correct and intended? http://gerrit.cloudera.org:8080/#/c/11084/1/src/kudu/tools/kudu-tool-test.cc@1514 PS1, Line 1514: nit: the same question on the spacing -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 23:23:00 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Atomicize OpId and Timestamp assignment
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11070 to look at the new patch set (#4). Change subject: WIP: Atomicize OpId and Timestamp assignment .. WIP: Atomicize OpId and Timestamp assignment This is a simpler version of a previous patch but without big changes to the queue or consensus. Change-Id: I0d369423dd5c96b653b424c29744507edd874357 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java M src/kudu/consensus/consensus_meta-test.cc M src/kudu/consensus/pending_rounds.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager.cc 8 files changed, 77 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/11070/4 -- To view, visit http://gerrit.cloudera.org:8080/11070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357 Gerrit-Change-Number: 11070 Gerrit-PatchSet: 4 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms-tool: lookup master addresses config from master
Hello Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11083 to look at the new patch set (#2). Change subject: hms-tool: lookup master addresses config from master .. hms-tool: lookup master addresses config from master The HMS check and fix tools currently use the master addresses configuration provided on the command line to validate and fix HMS table metadata. This only works correctly if the administrator inputs the master addresses exactly as its configured on the masters. If the hostports are reordered or use a slightly different format which resolves to the same addresses, then metadata will be flagged as stale and rewritten unnecessarily. This commit changes the handling so that the tools use the master addresses returned from the ConnectToCluster mechanism already present in the client, which should match the master addresses config of the master exactly. Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_hms.cc 6 files changed, 55 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/11083/2 -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms-tool: lookup master addresses config from master
Dan Burkert has removed Alexey Serbin from this change. ( http://gerrit.cloudera.org:8080/11083 ) Change subject: hms-tool: lookup master addresses config from master .. Removed reviewer Alexey Serbin. -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hms-tool: lookup master addresses config from master
Hello Alexey Serbin, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11083 to review the following change. Change subject: hms-tool: lookup master addresses config from master .. hms-tool: lookup master addresses config from master The HMS check and fix tools currently use the master addresses configuration provided on the command line to validate and fix HMS table metadata. This only works correctly if the administrator inputs the master addresses exactly as its configured on the masters. If the hostports are reordered or use a slightly different format which resolves to the same addresses, then metadata will be flagged as stale and rewritten unnecessisarily. This commit changes the handling so that the tools use the master addresses returned from the ConnectToCluster mechanism already present in the client, so that the master addresses config will match the masters. Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_hms.cc 6 files changed, 55 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/11083/1 -- To view, visit http://gerrit.cloudera.org:8080/11083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If170cebe6e5d7fa05fbd7faf18755aa57bdfeeec Gerrit-Change-Number: 11083 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hao Hao
[kudu-CR] perf loadgen: add --auto database flag and change auto table name generation
Hello Alexey Serbin, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11084 to review the following change. Change subject: perf loadgen: add --auto_database flag and change auto table name generation .. perf loadgen: add --auto_database flag and change auto table name generation This commit changes the 'perf loadgen' tool to create HMS-compliant tables when an auto table is created. A new flag allows the database to be configured, and defaults to the 'default' HMS database. Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_perf.cc 2 files changed, 22 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11084/1 -- To view, visit http://gerrit.cloudera.org:8080/11084 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I20654f7c1525f3804c5f307d90d23e43b3882e8a Gerrit-Change-Number: 11084 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hao Hao
[kudu-CR] KUDU-2191: Correct error handling in Metadata upgrade tool
Hao Hao has abandoned this change. ( http://gerrit.cloudera.org:8080/10728 ) Change subject: KUDU-2191: Correct error handling in Metadata upgrade tool .. Abandoned No longer apply. -- To view, visit http://gerrit.cloudera.org:8080/10728 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ie1d3ae49e03f5c3cc809846fee4fcde885aff25f Gerrit-Change-Number: 10728 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2507: use FBM for KuduMiniCluster
Andrew Wong has abandoned this change. ( http://gerrit.cloudera.org:8080/10961 ) Change subject: KUDU-2507: use FBM for KuduMiniCluster .. Abandoned Issue fixed via other patch -- To view, visit http://gerrit.cloudera.org:8080/10961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I8c2622d7049dacf7693e7f830fa022dd6c7d7274 Gerrit-Change-Number: 10961 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Scala code formatting with Scalafmt
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11030 ) Change subject: Scala code formatting with Scalafmt .. Scala code formatting with Scalafmt Scalafmt https://scalameta.org/scalafmt/#Scalafmt is a code formatting tool, scalafmt.conf is the main configuration file. Scalafmt is added using the Gradle Scalafmt plugin: https://github.com/alenkacz/gradle-scalafmt and Maven scalafmt plugin: https://github.com/SimonJPegg/mvn_scalafmt. The plugin is configured to run on compile or can invoked with 'gradle scalafmt' or 'gradle testScalafmt' to format test code. It will run during the verification stage of a Maven build. Set gradle endoding globally because 'testScalafmt' target would otherwise choke on some special characters. Change-Id: Iac96383d88394084e19712177d05f9fc63de766c Reviewed-on: http://gerrit.cloudera.org:8080/11030 Tested-by: Kudu Jenkins Reviewed-by: Tony Foerster Reviewed-by: Grant Henke --- M java/.gitignore A java/.scalafmt.conf M java/buildSrc/build.gradle M java/gradle.properties M java/gradle/quality.gradle M java/kudu-backup/pom.xml M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackupRDD.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestoreOptions.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-spark-tools/pom.xml M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala M java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala M java/kudu-spark/pom.xml M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduWriteOptions.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/package.scala M java/pom.xml 24 files changed, 1,117 insertions(+), 767 deletions(-) Approvals: Kudu Jenkins: Verified Tony Foerster: Looks good to me, but someone else must approve Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11030 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iac96383d88394084e19712177d05f9fc63de766c Gerrit-Change-Number: 11030 Gerrit-PatchSet: 14 Gerrit-Owner: Tony Foerster Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tony Foerster
[kudu-CR] KUDU-2507: use FBM for KuduMiniCluster
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10961 ) Change subject: KUDU-2507: use FBM for KuduMiniCluster .. Patch Set 1: I don't think we need this now that a TEST_TMPDIR fix is in https://github.com/apache/kudu/commits/c2cecb0 -- To view, visit http://gerrit.cloudera.org:8080/10961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c2622d7049dacf7693e7f830fa022dd6c7d7274 Gerrit-Change-Number: 10961 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 30 Jul 2018 20:09:55 + Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 ) Change subject: hms precheck tool .. Patch Set 6: Verified+1 unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/11040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e Gerrit-Change-Number: 11040 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 19:46:33 + Gerrit-HasComments: No
[kudu-CR] hms precheck tool
Dan Burkert has removed a vote on this change. Change subject: hms precheck tool .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e Gerrit-Change-Number: 11040 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Add extra info about using multiple clients
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10962 ) Change subject: [java] Add extra info about using multiple clients .. [java] Add extra info about using multiple clients The AsyncKuduClient and KuduClient are meant to map 1-1 with Kudu clusters used by an application. In particular, an application shouldn't make more than one client per cluster. This patch adds a note to this effect to the AsyncKuduClient javadoc. Additionally, some use cases require using multiple clients in multiple applications running on the same machine. For example, to provide very strict classpath isolation, or to run applications on different JVM versions, or even for extra security by isolating different data flows in different processes. In this case, I've seen client instances put pressure on thread limits on machines with a lot of cores because the AsyncKuduClient starts at least (2 * #cpu) threads for a netty threadpool. This patch also adds advice to adjust the netty worker count to help users with this case. Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89 Reviewed-on: http://gerrit.cloudera.org:8080/10962 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java 1 file changed, 14 insertions(+), 4 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89 Gerrit-Change-Number: 10962 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] Kudu-1291 Efficiently support predicates on non-prefix key components
Anupama Gupta has posted comments on this change. (
http://gerrit.cloudera.org:8080/10983 )
Change subject: Kudu-1291 Efficiently support predicates on non-prefix key
components
..
Patch Set 4:
(41 comments)
Thanks for the helpful comments Mike. Please review the changes.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/cfile/cfile_reader.cc@793
PS3, Line 793:
> This only works if the cell is a pointer, like in the case of a string valu
Yes, this works for string and binary values stored as uint8_t byte array. The
ad-hoc index stores the keys that are either encoded as binary prefix blocks or
binary dict blocks.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/encoded_key.h
File src/kudu/common/encoded_key.h:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/encoded_key.h@63
PS3, Line 63: um_columns = 0)
> document this parameter in the comment
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/encoded_key.cc
File src/kudu/common/encoded_key.cc:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/encoded_key.cc@111
PS3, Line 111: num_columns, arena)) {
> I thought we were going to generalize this to any column in the compound PK
Right, I have updated this now.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/partial_row.h@494
PS3, Line 494: rivate:
> This class is a part of the public C++ client API (see KUDU_EXPORT) so we w
Thanks, it makes sense. I have now added the calling class (CfileSet) as a
friend class of KuduPartialRow.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/partial_row.h@498
PS3, Line 498: friend class RowOpe
> Why is this private field now public? Should not be necessary.
This field is used in the function - CFileSet::Iterator::PrepareBatch , to set
the minimum values for some columns. I am not quite sure if there is a better
alternative to this.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/wire_protocol-test-util.h
File src/kudu/common/wire_protocol-test-util.h:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/common/wire_protocol-test-util.h@38
PS3, Line 38: inline void
AddTestRowWithNullableStringToPB(RowOperationsPB::Type op_type,
> move this to the test where it's being used
I have removed this now.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.h
File src/kudu/tablet/cfile_set.h:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.h@203
PS3, Line 203:
: // This function is used to place the va
> These methods need doc comments
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.h@230
PS3, Line 230: initted_(false),
> add documentation for this method
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.h@264
PS3, Line 264: size_t cur_idx_;
: size_t prepared_count_;
> these variables should be documented with a comment
Done
http://gerrit.cloudera.org:8080/#/c/10983/1/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:
http://gerrit.cloudera.org:8080/#/c/10983/1/src/kudu/tablet/cfile_set.cc@669
PS1, Line 669:
> instead of this loop, I wonder if we can instead "peek" at the last value s
Makes sense. But I am not quite sure if we can "peek" at the last seeked
ordinal in the ad-hoc index column here.
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@395
PS3, Line 395: tate.
> let's rename this CanUseSkipScan(), have it return a boolean, and set
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@397
PS3, Line 397: }
> Add a flag DEFINE_bool(enable_skip_scan, true, "...") that allows us to tur
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@400
PS3, Line 400: int num_key_cols =
base_data_->tablet_schema().num_key_columns();
> looks like a compile error? probably some unstaged changes
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@415
PS3, Line 415: for (auto it=predicates.begin(); it!=predicates.end(); ++it) {
> error: use of undeclared identifier 'suffix_key_id_' [clang-diagnostic-erro
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@418
PS3, Line 418: string col_name = (it->second).column().name();
> error: use of undeclared identifier 'suffix_key_id_' [clang-diagnostic-erro
Done
http://gerrit.cloudera.org:8080/#/c/10983/3/src/kudu/tablet/cfile_set.cc@429
PS3, Line 429: nonfirst_key_pred_exists = true;
> error: use of undeclared identifier 'suffix_key_id_'; did
[kudu-CR] Kudu-1291 Efficiently support predicates on non-prefix key components
Hello Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10983 to look at the new patch set (#4). Change subject: Kudu-1291 Efficiently support predicates on non-prefix key components .. Kudu-1291 Efficiently support predicates on non-prefix key components This patch implements index skip scan approach in the case when there exists an equality predicate on any of the non-first primary key columns. This feature uses the following approach: 1. Seek at the first unique prefix key in the ad-hoc index tree. 2. Seek to the relevant entry corresponding to the unique prefix key found in 1. and the predicate value on the suffix key column. 3. To handle the case where multiple entries exist with respect to a unique prefix key, store the upper bound index of these entries. 4. Repeat steps 1-3, till all the unique prefix keys are scanned. Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 889 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/10983/4 -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 4 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#10). Change subject: WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column .. WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 889 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/10 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 10 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] Kudu-1291 Efficiently support predicates on non-prefix key components
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#9). Change subject: Kudu-1291 Efficiently support predicates on non-prefix key components .. Kudu-1291 Efficiently support predicates on non-prefix key components This patch implements index skip scan approach in the case when there exists an equality predicate on any of the non-first primary key columns. This feature uses the following approach: 1. Seek at the first unique prefix key in the ad-hoc index tree. 2. Seek to the relevant entry corresponding to the unique prefix key found in 1. and the predicate value on the suffix key column. 3. To handle the case where multiple entries exist with respect to a unique prefix key, store the upper bound index of these entries. 4. Repeat steps 1-3, till all the unique prefix keys are scanned. Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 890 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/9 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11018 ) Change subject: hms-tool: refactor check tool and combine upgrade and fix .. hms-tool: refactor check tool and combine upgrade and fix This commit combines the 'hms upgrade' and 'hms fix' tools. The tools previously had overlapping responsibilities for checking some types of inconsistencies. The 'fix' tool now has flags which can control whether it attempts certain types of fixes: --drop_orphan_hms_tables=false --create_missing_hms_tables=true --fix_inconsistent_tables=true --upgrade_hms_tables=true `drop_orphan_hms_tables` defaults to false since it deletes Hive tables, and the tool can not rule-out that they are being used. Additionally, a --dryrun flag is provided in order to print steps that would be taken without actually making modifications. The checking logic has been expanded to account for more potential inconsistencies, and where possible the fix tool now can automatically repair these inconsistencies. Finally, the input prompt and default database functionality has been removed in order to simplify the tool. Instead, the check tool will issue hints including instructions for how to rename tables without a database or with a Hive-incompatible name using the 'kudu table rename_table' tool. We can always add this functionality back in the future if we determine it helps out users. Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Reviewed-on: http://gerrit.cloudera.org:8080/11018 Tested-by: Dan Burkert Reviewed-by: Hao Hao --- M src/kudu/gutil/strings/escaping.h M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 6 files changed, 887 insertions(+), 818 deletions(-) Approvals: Dan Burkert: Verified Hao Hao: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 12 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 ) Change subject: hms-tool: refactor check tool and combine upgrade and fix .. Patch Set 11: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@169 PS10, Line 169: PrintTables > Not entirely sure I follow, but PrintKuduTables can't call into PrintTables I was thinking when print Kudu tables, the hms table can be left as blank. But after a second thought, having a separate method for printing Kudu table alone probably is more clear. So please ignore my previous comment. http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@415 PS10, Line 415: endl : << "Sug > It's somewhat complicated to do that because of the colon in the first case I see, that's OK then. -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 11 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 30 Jul 2018 18:43:33 + Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11018 ) Change subject: hms-tool: refactor check tool and combine upgrade and fix .. Patch Set 11: Verified+1 unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 11 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 30 Jul 2018 18:23:05 + Gerrit-HasComments: No
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Dan Burkert has removed a vote on this change. Change subject: hms-tool: refactor check tool and combine upgrade and fix .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 11 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#8). Change subject: WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column .. WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 890 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/8 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 8 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#7). Change subject: WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column .. WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 891 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/7 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 7 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#6). Change subject: WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column .. WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 890 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/6 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 6 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11067 to look at the new patch set (#5). Change subject: WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column .. WIP 1. Added correctness test for skip scan 2. Extended the approach for equality query on any PK column Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 9 files changed, 890 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/11067/5 -- To view, visit http://gerrit.cloudera.org:8080/11067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffbc240233e5ad52f6a1c7912d4e44baf32a5f2d Gerrit-Change-Number: 11067 Gerrit-PatchSet: 5 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11018 )
Change subject: hms-tool: refactor check tool and combine upgrade and fix
..
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@169
PS10, Line 169: PrintTables
> It looks like PrintKuduTables can also be embedded into this function?
Not entirely sure I follow, but PrintKuduTables can't call into PrintTables
without a change in behavior, because PrintTables shows a different set of
columns.
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@366
PS10, Line 366: Suggestion: rename the Kudu table(s) to be Hive-compatible,
> Should we call out this needs to be run before running the fix tool. I am g
Done
http://gerrit.cloudera.org:8080/#/c/11018/10/src/kudu/tools/tool_action_hms.cc@415
PS10, Line 415: endl
: << "Sug
> nit: this can be put upfront L408 to avoid dup code.
It's somewhat complicated to do that because of the colon in the first case:
cout << endl
<< "Suggestion: use the fix tool to repair the Kudu and Hive Metastore
catalog metadata";
if (report.orphan_hms_tables.empty()) {
cout << ":" << endl << "\t$ kudu hms fix " << master_addrs << endl;
} else {
cout << endl
<< "and drop Hive Metastore table(s) which reference non-existent
Kudu tables:" << endl
<< "\t$ kudu hms fix --drop_orphan_hms_tables " << master_addrs <<
endl;
}
I think it reads better as is, but I can switch it if you feel strongly.
--
To view, visit http://gerrit.cloudera.org:8080/11018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f
Gerrit-Change-Number: 11018
Gerrit-PatchSet: 11
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:34:21 +
Gerrit-HasComments: Yes
[kudu-CR] [java] Add extra info about using multiple clients
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10962 ) Change subject: [java] Add extra info about using multiple clients .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10962/1/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/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@86 PS1, Line 86: : * : * In rare cases where a single app > Are you suggesting making those two points in a bulleted list? I really don Nope, separating out into two sections as you did was pretty much what I had in mind. -- To view, visit http://gerrit.cloudera.org:8080/10962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89 Gerrit-Change-Number: 10962 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 30 Jul 2018 17:30:57 + Gerrit-HasComments: Yes
[kudu-CR] WIP tool to rewrite cfiles
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11078 Change subject: WIP tool to rewrite cfiles .. WIP tool to rewrite cfiles Adds a tool to rewrite cfiles while skipping cblocks that have been corrupted. This cannot be the final solution, since any positional indexing on the CFile is ruined due to the skipped cblocks, but it's a start. Nullable and key CFiles are not supported. Example: $ kudu file rewrite_cfile src_cfile dst_cfile WIP: in skipping corrupted blocks, we're not doing some accounting of last_prepared_idx_, leading to a DCHECK failure, though I think this works as expected for a healthy block. Change-Id: I55a52b7a237e99d0e66218e02c213d4a9a7781e7 --- M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_util.cc M src/kudu/cfile/cfile_util.h M src/kudu/cfile/cfile_writer.cc M src/kudu/fs/block_manager.h M src/kudu/tools/tool_action_file.cc 7 files changed, 330 insertions(+), 99 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/11078/1 -- To view, visit http://gerrit.cloudera.org:8080/11078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I55a52b7a237e99d0e66218e02c213d4a9a7781e7 Gerrit-Change-Number: 11078 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] [java] Add extra info about using multiple clients
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/10962 )
Change subject: [java] Add extra info about using multiple clients
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10962/1/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/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@83
PS1, Line 83: .
> nit: not your fault, but comma
Done
http://gerrit.cloudera.org:8080/#/c/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@85
PS1, Line 85: that
> not from this changelist, but just a small nit: that --> when (take it with
Done
http://gerrit.cloudera.org:8080/#/c/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@86
PS1, Line 86: It's generally
: * not appropriate to use multiple client instances connected to
the same
: * cluster in a single application.
> nit: maybe move this before "Separate client instances should be..." and a
Are you suggesting making those two points in a bulleted list? I really don't
like that because the points aren't parallel. I reworked the section to be
briefer.
http://gerrit.cloudera.org:8080/#/c/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@91
PS1, Line 91: See the options in {@link AsyncKuduClientBuilder}
> Maybe, it's worth pointing to KuduClientBuilder as well?
The sync client and builder refer to their async counterparts for docs.
http://gerrit.cloudera.org:8080/#/c/10962/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2267
PS1, Line 2267: appropriately
> nit: It wasn't super obvious what "appropriately" meant here until I read t
Done
--
To view, visit http://gerrit.cloudera.org:8080/10962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89
Gerrit-Change-Number: 10962
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Mon, 30 Jul 2018 16:55:24 +
Gerrit-HasComments: Yes
[kudu-CR] hms-tool: refactor check tool and combine upgrade and fix
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11018 to look at the new patch set (#11). Change subject: hms-tool: refactor check tool and combine upgrade and fix .. hms-tool: refactor check tool and combine upgrade and fix This commit combines the 'hms upgrade' and 'hms fix' tools. The tools previously had overlapping responsibilities for checking some types of inconsistencies. The 'fix' tool now has flags which can control whether it attempts certain types of fixes: --drop_orphan_hms_tables=false --create_missing_hms_tables=true --fix_inconsistent_tables=true --upgrade_hms_tables=true `drop_orphan_hms_tables` defaults to false since it deletes Hive tables, and the tool can not rule-out that they are being used. Additionally, a --dryrun flag is provided in order to print steps that would be taken without actually making modifications. The checking logic has been expanded to account for more potential inconsistencies, and where possible the fix tool now can automatically repair these inconsistencies. Finally, the input prompt and default database functionality has been removed in order to simplify the tool. Instead, the check tool will issue hints including instructions for how to rename tables without a database or with a Hive-incompatible name using the 'kudu table rename_table' tool. We can always add this functionality back in the future if we determine it helps out users. Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f --- M src/kudu/gutil/strings/escaping.h M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_catalog.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_hms.cc 6 files changed, 887 insertions(+), 818 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/11018/11 -- To view, visit http://gerrit.cloudera.org:8080/11018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieee478002eb56a278cfd74255670baaf28109b8f Gerrit-Change-Number: 11018 Gerrit-PatchSet: 11 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [java] Add extra info about using multiple clients
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10962 to look at the new patch set (#2). Change subject: [java] Add extra info about using multiple clients .. [java] Add extra info about using multiple clients The AsyncKuduClient and KuduClient are meant to map 1-1 with Kudu clusters used by an application. In particular, an application shouldn't make more than one client per cluster. This patch adds a note to this effect to the AsyncKuduClient javadoc. Additionally, some use cases require using multiple clients in multiple applications running on the same machine. For example, to provide very strict classpath isolation, or to run applications on different JVM versions, or even for extra security by isolating different data flows in different processes. In this case, I've seen client instances put pressure on thread limits on machines with a lot of cores because the AsyncKuduClient starts at least (2 * #cpu) threads for a netty threadpool. This patch also adds advice to adjust the netty worker count to help users with this case. Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java 1 file changed, 14 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/10962/2 -- To view, visit http://gerrit.cloudera.org:8080/10962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I34242862ee6d806b74d7717399e5fd0260dece89 Gerrit-Change-Number: 10962 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.6.x) KUDU-2510 Fix symmetric difference logging
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11034 ) Change subject: KUDU-2510 Fix symmetric difference logging .. KUDU-2510 Fix symmetric difference logging When the on-disk and provided master_addresses don't match, the error message was misleading as it showed them swapped, i.e. the --master_addresses lists 3 masters, and there's 1 in the Raft config, it showed "on-disk master list (master1:7051, master2:7051, master3:7051) and provided master list (master1:7051) differ." This commit swaps these two lists. Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Reviewed-on: http://gerrit.cloudera.org:8080/11031 Reviewed-by: Will Berkeley Tested-by: Kudu Jenkins (cherry picked from commit 7aab411d3187ea066c935af0215f894c5eca6aae) Reviewed-on: http://gerrit.cloudera.org:8080/11034 --- M src/kudu/master/sys_catalog.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: merged Gerrit-Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Gerrit-Change-Number: 11034 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.7.x) KUDU-2510 Fix symmetric difference logging
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11033 ) Change subject: KUDU-2510 Fix symmetric difference logging .. KUDU-2510 Fix symmetric difference logging When the on-disk and provided master_addresses don't match, the error message was misleading as it showed them swapped, i.e. the --master_addresses lists 3 masters, and there's 1 in the Raft config, it showed "on-disk master list (master1:7051, master2:7051, master3:7051) and provided master list (master1:7051) differ." This commit swaps these two lists. Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Reviewed-on: http://gerrit.cloudera.org:8080/11031 Reviewed-by: Will Berkeley Tested-by: Kudu Jenkins (cherry picked from commit 7aab411d3187ea066c935af0215f894c5eca6aae) Reviewed-on: http://gerrit.cloudera.org:8080/11033 --- M src/kudu/master/sys_catalog.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11033 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: merged Gerrit-Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Gerrit-Change-Number: 11033 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.7.x) KUDU-2510 Fix symmetric difference logging
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11033 ) Change subject: KUDU-2510 Fix symmetric difference logging .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11033 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: comment Gerrit-Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Gerrit-Change-Number: 11033 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 30 Jul 2018 16:40:33 + Gerrit-HasComments: No
[kudu-CR](branch-1.6.x) KUDU-2510 Fix symmetric difference logging
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11034 ) Change subject: KUDU-2510 Fix symmetric difference logging .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.6.x Gerrit-MessageType: comment Gerrit-Change-Id: I1fd5a0aaa5bd1398d874a8526240b12c37baad70 Gerrit-Change-Number: 11034 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 30 Jul 2018 16:40:36 + Gerrit-HasComments: No
[kudu-CR] [KUDU-2521] Java Implementation for BloomFilter
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11077 )
Change subject: [KUDU-2521] Java Implementation for BloomFilter
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11077/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java
File java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java:
http://gerrit.cloudera.org:8080/#/c/11077/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@22
PS1, Line 22: public class BloomFilter {
Is this meant to be part of the public API or an internal-facing class? I'm
guessing that consumers of the kudu client need to actually construct the bloom
filter in some way, so this is public, right?
Either way, we should add an InterfaceAudience and InterfaceStability
annotation.
http://gerrit.cloudera.org:8080/#/c/11077/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@40
PS1, Line 40: genBloomKeyProbe
this pattern seems like it will create a new object for each lookup in the
bloom filter. Although these are short-lived objects, I'm not sure if we can
count on them being removed by escape analysis. Maybe instead we can make
BloomKeyProbe a reusable mutable object?
http://gerrit.cloudera.org:8080/#/c/11077/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@86
PS1, Line 86: CharsetUtil
I think we can use StandardCharsets from java 7 here
http://gerrit.cloudera.org:8080/#/c/11077/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@139
PS1, Line 139: CityHash
I'm curious why the decision to use CityHash here? We are using murmur hashes
for partitioning, and internal to the tserver using CityHash for blooms on PK
lookups.
Is there an advantage to this bloom filter matching the hash used by the blooms
on the server side? I can't quite see how it would be used. If the choice of
hash is arbitrary, maybe better to just stick with Murmur since we already
included the library? Or maybe we can benchmark some alternatives for this use
case and see which is best? I think performance on small strings is most
important, even if the hash function is lower "quality". See
https://bigdata.uni-saarland.de/publications/p249-richter.pdf for some
interesting discussion
Is there any way we can use the bloom filters to eliminate entire partitions
while scanning if the bloom is calculated on the partitioning key? I'm not sure
if the bloom could be used directly, but I wonder if this would be useful for
joins in some cases (eg by just calculating a simple bitmap of matching
partitions based on knowledge of the hash partitioning on the build side of the
join)
--
To view, visit http://gerrit.cloudera.org:8080/11077
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50c5dce4d24707c068c5ab94e5d311b12b3251b8
Gerrit-Change-Number: 11077
Gerrit-PatchSet: 1
Gerrit-Owner: jinxing6...@126.com
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 30 Jul 2018 16:05:08 +
Gerrit-HasComments: Yes
[kudu-CR] [docs] Add changing master hostnames workflow
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11058 ) Change subject: [docs] Add changing master hostnames workflow .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11058/1/docs/administration.adoc File docs/administration.adoc: http://gerrit.cloudera.org:8080/#/c/11058/1/docs/administration.adoc@676 PS1, Line 676: new-master-name-1 > I agree it should be FQDN here (for best practices's sake), and maybe it's added .example.com to all hostnames to indicate this http://gerrit.cloudera.org:8080/#/c/11058/1/docs/administration.adoc@679 PS1, Line 679: master_addresses > Right, that was my point -- if IP addresses changed, I though it would be n Done http://gerrit.cloudera.org:8080/#/c/11058/1/docs/administration.adoc@681 PS1, Line 681: tserver_master_addrs > same as above Done -- To view, visit http://gerrit.cloudera.org:8080/11058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0df87d5e294d8b7bf5c7b8f94a63599ffd7ebe03 Gerrit-Change-Number: 11058 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 30 Jul 2018 15:43:42 + Gerrit-HasComments: Yes
[kudu-CR] [kudu-admin-test] improvements on error reporting
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11046 ) Change subject: [kudu-admin-test] improvements on error reporting .. Patch Set 3: Verified+1 Unrelated flakes in: kudu.tests.test_scantoken.TestScanToken.test_double_pred kudu.tests.test_scantoken.TestScanToken.test_unixtime_micros_pred org.apache.kudu.backup.TestKuduBackup.Backup and Restore Multiple Tables TlsSocketTest.TestRecvFailure org.apache.kudu.backup.TestKuduBackup.Random Backup and Restore -- To view, visit http://gerrit.cloudera.org:8080/11046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfa74d5a78e89bf7ff1a0e914384999466460145 Gerrit-Change-Number: 11046 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 30 Jul 2018 15:06:42 + Gerrit-HasComments: No
[kudu-CR] [kudu-admin-test] improvements on error reporting
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11046 ) Change subject: [kudu-admin-test] improvements on error reporting .. [kudu-admin-test] improvements on error reporting This changelist introduces improvements on logging in case if an assertion triggers in the rebalancer-related scenarios. I found it's hard to troubleshoot the flakiness reported lately in the RebalancerAndSingleReplicaTablets.SingleReplicasStayOrMove scenario. With hindsight, it turned out that improved logging on assertions would help a lot. Change-Id: Ibfa74d5a78e89bf7ff1a0e914384999466460145 Reviewed-on: http://gerrit.cloudera.org:8080/11046 Reviewed-by: Andrew Wong Tested-by: Alexey Serbin --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 47 insertions(+), 33 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/11046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ibfa74d5a78e89bf7ff1a0e914384999466460145 Gerrit-Change-Number: 11046 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Mike Percy
[kudu-CR] [kudu-admin-test] improvements on error reporting
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11046 ) Change subject: [kudu-admin-test] improvements on error reporting .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11046 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ibfa74d5a78e89bf7ff1a0e914384999466460145 Gerrit-Change-Number: 11046 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Mike Percy
[kudu-CR] [KUDU-2521] Java Implementation for BloomFilter
jinxing6...@126.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11077 Change subject: [KUDU-2521] Java Implementation for BloomFilter .. [KUDU-2521] Java Implementation for BloomFilter Change-Id: I50c5dce4d24707c068c5ab94e5d311b12b3251b8 --- A java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java A java/kudu-client/src/main/java/org/apache/kudu/util/CityHash.java A java/kudu-client/src/test/java/org/apache/kudu/client/TestBloomFilter.java 3 files changed, 673 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11077/1 -- To view, visit http://gerrit.cloudera.org:8080/11077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I50c5dce4d24707c068c5ab94e5d311b12b3251b8 Gerrit-Change-Number: 11077 Gerrit-PatchSet: 1 Gerrit-Owner: jinxing6...@126.com
[kudu-CR] test
jinxing6...@126.com has abandoned this change. ( http://gerrit.cloudera.org:8080/11076 ) Change subject: test .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/11076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ia2e23eebda31743032e38c2f5c7c2a88ccdd2611 Gerrit-Change-Number: 11076 Gerrit-PatchSet: 1 Gerrit-Owner: jinxing6...@126.com Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] test
jinxing6...@126.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11076 Change subject: test .. test Change-Id: Ia2e23eebda31743032e38c2f5c7c2a88ccdd2611 --- A test 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11076/1 -- To view, visit http://gerrit.cloudera.org:8080/11076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia2e23eebda31743032e38c2f5c7c2a88ccdd2611 Gerrit-Change-Number: 11076 Gerrit-PatchSet: 1 Gerrit-Owner: jinxing6...@126.com
