[kudu-CR] [tablet server-test] cleaner exit-on-failure in TestStatus
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11506 ) Change subject: [tablet_server-test] cleaner exit-on-failure in TestStatus .. Patch Set 1: Verified+1 Unrelated flake in HmsConfigurations/AlterTableRandomized.TestRandomSequence/0 -- To view, visit http://gerrit.cloudera.org:8080/11506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c5efef9e579f6fd357357517c32a26440896967 Gerrit-Change-Number: 11506 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Comment-Date: Tue, 25 Sep 2018 04:27:59 + Gerrit-HasComments: No
[kudu-CR] [tablet server-test] cleaner exit-on-failure in TestStatus
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11506 ) Change subject: [tablet_server-test] cleaner exit-on-failure in TestStatus .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I7c5efef9e579f6fd357357517c32a26440896967 Gerrit-Change-Number: 11506 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo
[kudu-CR] [location awareness] Add location info in ksck report
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 ) Change subject: [location_awareness] Add location info in ksck report .. Patch Set 5: > Uploaded patch set 5. It seems RemoteKsckTest.TestClusterWithLocation is still failing. -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 5 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 25 Sep 2018 04:22:14 + Gerrit-HasComments: No
[kudu-CR] [tablet server-test] cleaner exit-on-failure in TestStatus
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11506 Change subject: [tablet_server-test] cleaner exit-on-failure in TestStatus .. [tablet_server-test] cleaner exit-on-failure in TestStatus Updated the TestStatus scenario of the TabletServerTest to terminate cleaner and faster in case of a failure when restarting tablet server. Prior to this fix, when the mini tablet server fails to restart, the test scenario would run indefinitely or until some higher-level test framework terminates the tablet_server-test process. As for the reason behind the failure on restart, the mini tablet server might fail to restart if one the ports it tries to bind to is used by another process. Change-Id: I7c5efef9e579f6fd357357517c32a26440896967 --- M src/kudu/tserver/tablet_server-test.cc 1 file changed, 1 insertion(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/11506/1 -- To view, visit http://gerrit.cloudera.org:8080/11506 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7c5efef9e579f6fd357357517c32a26440896967 Gerrit-Change-Number: 11506 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] Add Hive Metastore service principal configuration
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11503 ) Change subject: Add Hive Metastore service principal configuration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11503/1/src/kudu/thrift/client.h File src/kudu/thrift/client.h: http://gerrit.cloudera.org:8080/#/c/11503/1/src/kudu/thrift/client.h@56 PS1, Line 56: > Can you doc what it means to leave this empty (i.e. the default value)? I p Done -- To view, visit http://gerrit.cloudera.org:8080/11503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 Gerrit-Change-Number: 11503 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 25 Sep 2018 00:52:52 + Gerrit-HasComments: Yes
[kudu-CR] Add Hive Metastore service principal configuration
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11503 to look at the new patch set (#2). Change subject: Add Hive Metastore service principal configuration .. Add Hive Metastore service principal configuration This commit adds a new flag, --hive_metastore_kerberos_principal flag which corresponds to the HMS 'hive.metastore.kerberos.principal' configuration. This configuration is rarely overridden, but in cases where it is, having a way to match it in Kudu is critical. Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 --- M src/kudu/hms/hms_catalog-test.cc M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_client-test.cc M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/thrift/client.cc M src/kudu/thrift/client.h M src/kudu/thrift/sasl_client_transport.cc M src/kudu/thrift/sasl_client_transport.h M src/kudu/tools/kudu-tool-test.cc 10 files changed, 36 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/11503/2 -- To view, visit http://gerrit.cloudera.org:8080/11503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 Gerrit-Change-Number: 11503 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add Hive Metastore service principal configuration
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11503 ) Change subject: Add Hive Metastore service principal configuration .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11503/1/src/kudu/thrift/client.h File src/kudu/thrift/client.h: http://gerrit.cloudera.org:8080/#/c/11503/1/src/kudu/thrift/client.h@56 PS1, Line 56: // The registered name of the service (Kerberos principal). Can you doc what it means to leave this empty (i.e. the default value)? I presume it must be set on a Kerberized cluster, right? If that's true, perhaps CreateClientProtocol should DCHECK if this is an empty string? -- To view, visit http://gerrit.cloudera.org:8080/11503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 Gerrit-Change-Number: 11503 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 25 Sep 2018 00:05:21 + Gerrit-HasComments: Yes
[kudu-CR] Add Hive Metastore service principal configuration
Hello Adar Dembo, Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11503 to review the following change. Change subject: Add Hive Metastore service principal configuration .. Add Hive Metastore service principal configuration This commit adds a new flag, --hive_metastore_kerberos_principal flag which corresponds to the HMS 'hive.metastore.kerberos.principal' configuration. This configuration is rarely overridden, but in cases where it is, having a way to match it in Kudu is critical. Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 --- M src/kudu/hms/hms_catalog-test.cc M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_client-test.cc M src/kudu/thrift/client.cc M src/kudu/thrift/client.h M src/kudu/thrift/sasl_client_transport.cc M src/kudu/thrift/sasl_client_transport.h M src/kudu/tools/kudu-tool-test.cc 8 files changed, 30 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/11503/1 -- To view, visit http://gerrit.cloudera.org:8080/11503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 Gerrit-Change-Number: 11503 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 10: I'm planning on pushing this out and tweeting it out from @ApacheKudu tomorrow morning (9/25) California time. -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 10 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 23:32:29 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 10 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 23:31:40 + Gerrit-HasComments: No
[kudu-CR] [location awareness] Add location info in ksck report
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11422 to look at the new patch set (#5). Change subject: [location_awareness] Add location info in ksck report .. [location_awareness] Add location info in ksck report This patch adds additional tablet server location info into the ksck report. Before: Tablet Server Summary UUID | Address | Status --+-+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | HEALTHY ... After: Tablet Server Summary UUID | Address | Location| Status --+-+---+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | /foo-bar0/BAAZ.9-quux | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | | HEALTHY Location| Count ---+- | 3 /foo-bar0/BAAZ.9-quux | 1 ... Change-Id: Ideff2dd4975c99a1135002624de2620fb95c --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h 12 files changed, 117 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/11422/5 -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 5 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Hello Alexey Serbin, Mike Percy, Attila Bukor, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11263 to look at the new patch set (#10). Change subject: Blogpost describing index skip scan optimization. .. Blogpost describing index skip scan optimization. Link to the version with images: https://github.com/AnupamaGupta01/kudu/blob/blogpost-2/_posts/2018-09-25-index-skip-scan-optimization-in-kudu.md Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e --- A _posts/2018-09-25-index-skip-scan-optimization-in-kudu.md A img/index-skip-scan/example-table.png A img/index-skip-scan/skip-scan-example-table.png A img/index-skip-scan/skip-scan-performance-graph.png 4 files changed, 114 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/11263/10 -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 10 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Anupama Gupta has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: Sorry for the confusion. I have renamed the file name in the github version to make it consistent with the last updated change. The updated file name is '2018-09-25-index-skip-scan-optimization-in-kudu.md' -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 21:40:25 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: > Patch Set 9: > > > Patch Set 9: Verified-1 > > > > we should probably change the date in the filename as it would be below > > Mac's pipeline post otherwise, so I'm setting verified to -1 for now. > > Attila, I don't understand what you're saying here. Can you clarify? oh sorry. I meant that this post is dated 2018-08-17 and we already have a published one from 2018-09-11, meaning this post wouldn't go to the top, but to the second place instead. The file should simply be renamed to 2018-09-24-index-skip-scan-optimization-in-kudu.md to make sure it goes to the top. -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 21:23:53 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: > Patch Set 9: Verified-1 > > we should probably change the date in the filename as it would be below Mac's > pipeline post otherwise, so I'm setting verified to -1 for now. Attila, I don't understand what you're saying here. Can you clarify? -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 20:53:13 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: Verified-1 we should probably change the date in the filename as it would be below Mac's pipeline post otherwise, so I'm setting verified to -1 for now. -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 20:34:23 + Gerrit-HasComments: No
[kudu-CR] [location awareness] Add location info in ksck report
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11422 to look at the new patch set (#4). Change subject: [location_awareness] Add location info in ksck report .. [location_awareness] Add location info in ksck report This patch adds additional tablet server location info into the ksck report. Before: Tablet Server Summary UUID | Address | Status --+-+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | HEALTHY ... After: Tablet Server Summary UUID | Address | Location| Status --+-+---+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | /foo-bar0/BAAZ.9-quux | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | | HEALTHY Location| Count ---+- | 3 /foo-bar0/BAAZ.9-quux | 1 ... Change-Id: Ideff2dd4975c99a1135002624de2620fb95c --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h 12 files changed, 118 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/11422/4 -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 4 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] [location awareness] Add location info in ksck report
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 ) Change subject: [location_awareness] Add location info in ksck report .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/client.h@602 PS2, Line 602: /// @return The location of the tablet server. > What if there's no location? Done http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/tablet_server-internal.h File src/kudu/client/tablet_server-internal.h: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/tablet_server-internal.h@35 PS2, Line 35: // If not assigned, locatio > Please document what's implicit here: an empty string means no location is Done http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.h File src/kudu/tools/ksck_remote.h: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.h@89 PS2, Line 89: const HostPort &host_port, > warning: the const qualified parameter 'host_port' is copied for each invoc Done http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.cc@460 PS2, Line 460: > Put this on a separate line, please. Done -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 3 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 19:14:37 + Gerrit-HasComments: Yes
[kudu-CR] [location awareness] Add location info in ksck report
Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11422 to look at the new patch set (#3). Change subject: [location_awareness] Add location info in ksck report .. [location_awareness] Add location info in ksck report This patch adds additional tablet server location info into the ksck report. Before: Tablet Server Summary UUID | Address | Status --+-+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | HEALTHY ... After: Tablet Server Summary UUID | Address | Location| Status --+-+---+- 21b1960d8b164c869a3e600ed8bad1ea | 127.0.0.1:63000 | /foo-bar0/BAAZ.9-quux | HEALTHY 2c55bc50306a4463aa5a935aa97caff6 | 127.0.0.1:62973 | | HEALTHY a8dee6038c494b97b883d98292f7771f | 127.0.0.1:62983 | | HEALTHY d3664283918a434cbc5a8e067fa7c3b0 | 127.0.0.1:62978 | | HEALTHY Location| Count ---+- | 3 /foo-bar0/BAAZ.9-quux | 1 ... Change-Id: Ideff2dd4975c99a1135002624de2620fb95c --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h 12 files changed, 119 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/11422/3 -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 3 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 18:54:33 + Gerrit-HasComments: No
[kudu-CR] Implement BloomFilter Predicate in server side.
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11100 )
Change subject: Implement BloomFilter Predicate in server side.
..
Patch Set 8:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:
http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc@77
PS8, Line 77: Random rand(SeedRandom());
You shouldn't call this more than once per test. So maybe add it to the fixture?
http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/common/column_predicate-test.cc@1399
PS8, Line 1399: bfs.push_back(bf1);
Still got some usage of push_back in this test instead of emplace_back.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@275
PS6, Line 275:
: BloomFilterInner() : nhash_(0), hash_algorithm_(CITY_HASH) {}
:
: const Slice& bloom_data() const {
: return bloom_data_;
: }
:
: size_t nhash() const {
: return nhash_;
: }
:
: HashAlgorithmInBloomFilter hash_algorithm() const {
: return hash_algorithm_;
: }
:
: void set_nhash(size_t nhash) {
: nhash_ = nhash;
: }
:
: void set_bloom_data(Slice bloom_data) {
: bloom_data_ = bloom_data;
: }
:
: v
> Emmm I simply try to use setters and getters to let the calls look uniform.
Gotcha. Yeah, I agree, if you want to keep the setters/getters as an
abstraction, switch to a class so that default visibility is private. Or just
drop the setters/getters. Whatever you prefer.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.cc@275
PS6, Line 275: predicate_type_ = PredicateType::Equality;
> Yes, I just simply copy the Range simplify part. Should I modify the Range
Yes, please do.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/wire_protocol-test.cc@555
PS6, Line 555: TEST_F(WireProtocolTest,
TestColumnPredicateBloomFilterWithBound) {
> The BF test is juxtaposed with InList test above, It is necessary to do thi
Yeah, decomposing the one for InList would be great too.
Test code tends to be verbose and it's easy for our eyes to glaze over when
reviewing it, so anything you can do to make it shorter means we're more likely
to give it the same detail in code review as we would non-test code.
http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/util/bloom_filter.h
File src/kudu/util/bloom_filter.h:
http://gerrit.cloudera.org:8080/#/c/11100/8/src/kudu/util/bloom_filter.h@67
PS8, Line 67: case CITY_HASH: // Default use city hash.
Nit: don't need the comment here; it's obvious from the default value.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/util/bloom_filter.h
File src/kudu/util/bloom_filter.h:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/util/bloom_filter.h@57
PS6, Line 57: explicit BloomKeyProbe(const Slice &key,
HashAlgorithmInBloomFilter hash_algorithm = CITY_HASH)
> I chose to use constructor with default value. I can also simply delete the
Default values are fine too; I just wanted to make sure the repeated code is
consolidated.
--
To view, visit http://gerrit.cloudera.org:8080/11100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c
Gerrit-Change-Number: 11100
Gerrit-PatchSet: 8
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Mon, 24 Sep 2018 18:52:19 +
Gerrit-HasComments: Yes
[kudu-CR] [location awareness] Add location info in ksck report
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 ) Change subject: [location_awareness] Add location info in ksck report .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/client.h@602 PS2, Line 602: /// @return The location of the tablet server. What if there's no location? http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/tablet_server-internal.h File src/kudu/client/tablet_server-internal.h: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/client/tablet_server-internal.h@35 PS2, Line 35: const std::string location_ Please document what's implicit here: an empty string means no location is assigned. http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/11422/2/src/kudu/tools/ksck_remote.cc@460 PS2, Line 460: s->location() Put this on a separate line, please. -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 2 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 18:48:42 + Gerrit-HasComments: Yes
[kudu-CR] [location awareness] Add location info in ksck report
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 ) Change subject: [location_awareness] Add location info in ksck report .. Patch Set 2: > Uploaded patch set 2. Please address the following issues with your patch: 1) RemoteKsckTest.TestClusterWithLocation is failing 2) IWYU reports warnings -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624de2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 2 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 18:44:00 + Gerrit-HasComments: No
[kudu-CR] KUDU-2245 Graceful leadership transfer
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11251 ) Change subject: KUDU-2245 Graceful leadership transfer .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc File src/kudu/tools/tool_replica_util.cc: http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@310 PS10, Line 310: proxy > Maybe some naming cleanup or a bit of refactoring would help? 'from_ts_uui Yes, that was it -- it seems to be the reason behind my confusion. Indeed, the leader would change as per L284 in the original code. -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 10 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 17:59:58 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2245 Graceful leadership transfer
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11251 ) Change subject: KUDU-2245 Graceful leadership transfer .. Patch Set 11: > (9 comments) > > I tried doing a bit of a refactor on the CheckMoveComplete code but > maybe I do not understand it as well as I thought. Maybe we should > chat about it tomorrow? After looking at the step-by-step explanation I think it makes sense, yep. As we discussed offline, it makes sense to separate that refactoring into its own patch. That will be easier to track and review, especially given the fact that it's a bug. -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 11 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 17:58:10 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Blogpost describing index skip scan optimization.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11263 ) Change subject: Blogpost describing index skip scan optimization. .. Patch Set 9: Code-Review+2 Thanks! I'll work on getting this posted. -- To view, visit http://gerrit.cloudera.org:8080/11263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I2250652dcba3d1b0a06f1ffb7f23c11bf533d35e Gerrit-Change-Number: 11263 Gerrit-PatchSet: 9 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 24 Sep 2018 16:58:11 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck checksums: Add KsckChecksummer class
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11498 ) Change subject: [tools] ksck checksums: Add KsckChecksummer class .. Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.h File src/kudu/tools/ksck_checksum.h: http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.h@193 PS1, Line 193: Status ChecksumData(const KsckChecksumOptions& opts, > warning: function 'kudu::tools::KsckChecksummer::ChecksumData' has a defini Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@87 PS1, Line 87: const vector& table_filters, > warning: pass by value and use std::move [modernize-pass-by-value] Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@88 PS1, Line 88: const vector& tablet_id_filters) > warning: pass by value and use std::move [modernize-pass-by-value] Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@261 PS1, Line 261: shared_ptr ts = FindOrDie(cluster.tablet_servers(), > warning: the variable 'ts' is copy-constructed from a const reference but i Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@282 PS1, Line 282: std::move(replica_checksum)); > warning: passing result of std::move() as a const reference argument; no mo Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@286 PS1, Line 286: std::move(tablet_checksum)); > warning: passing result of std::move() as a const reference argument; no mo Done http://gerrit.cloudera.org:8080/#/c/11498/1/src/kudu/tools/ksck_checksum.cc@289 PS1, Line 289: InsertOrDie(table_checksum_map, table->name(), std::move(table_checksum)); > warning: passing result of std::move() as a const reference argument; no mo Done -- To view, visit http://gerrit.cloudera.org:8080/11498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa Gerrit-Change-Number: 11498 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 15:15:11 + Gerrit-HasComments: Yes
[kudu-CR] [tools] ksck checksums: Add KsckChecksummer class
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11498 to look at the new patch set (#2). Change subject: [tools] ksck checksums: Add KsckChecksummer class .. [tools] ksck checksums: Add KsckChecksummer class This removes the remaining checksum logic out of Ksck and into a new KsckChecksummer class, along with refactoring some parts of the logic into separate functions. There are no functional changes. The logic that was not refactored is directly related to KUDU-2179, and will be addressed in a follow-up patch. Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa --- M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_checksum.cc M src/kudu/tools/ksck_checksum.h 4 files changed, 343 insertions(+), 188 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/11498/2 -- To view, visit http://gerrit.cloudera.org:8080/11498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa Gerrit-Change-Number: 11498 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-686 (part 1/2): decompose guts of DMSIterator into DeltaPreparer
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/11394 ) Change subject: KUDU-686 (part 1/2): decompose guts of DMSIterator into DeltaPreparer .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26c92dfa69b5323e7fbb18d02010ed99cc29c3e5 Gerrit-Change-Number: 11394 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 24 Sep 2018 15:06:56 + Gerrit-HasComments: No
[kudu-CR] KUDU-686 (part 1/2): decompose guts of DMSIterator into DeltaPreparer
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/11394 ) Change subject: KUDU-686 (part 1/2): decompose guts of DMSIterator into DeltaPreparer .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11394/4/src/kudu/tablet/delta_store.h File src/kudu/tablet/delta_store.h: http://gerrit.cloudera.org:8080/#/c/11394/4/src/kudu/tablet/delta_store.h@133 PS4, Line 133: PreparedDeltas > This is an interesting idea, and I spent some time thinking about it. In th k, thanks for looking into it -- To view, visit http://gerrit.cloudera.org:8080/11394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26c92dfa69b5323e7fbb18d02010ed99cc29c3e5 Gerrit-Change-Number: 11394 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 24 Sep 2018 15:06:44 + Gerrit-HasComments: Yes
[kudu-CR] hybrid clock: restore SleepFor in WaitUntilAfterLocally
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/11495 ) Change subject: hybrid_clock: restore SleepFor in WaitUntilAfterLocally .. Patch Set 2: I guess I should have +2 before the push :) -- To view, visit http://gerrit.cloudera.org:8080/11495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecbb077b1f88293dcb0ab53e40ff3862f772694e Gerrit-Change-Number: 11495 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 24 Sep 2018 15:04:19 + Gerrit-HasComments: No
[kudu-CR] hybrid clock: restore SleepFor in WaitUntilAfterLocally
David Ribeiro Alves has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11495 ) Change subject: hybrid_clock: restore SleepFor in WaitUntilAfterLocally .. hybrid_clock: restore SleepFor in WaitUntilAfterLocally This seems to have gone missing from commit f2d96437c. The end result is a busy loop, which was probably unintentional. I also removed the loop, which seemed unnecessary with the SleepFor. To test, I wrote a small program that scanned a table at Now() + 10s, after setting --scanner_max_wait_ms to 1 (default is 1000). Although the wall clock time was the same, the cycle count was far higher per perf stat. Change-Id: Iecbb077b1f88293dcb0ab53e40ff3862f772694e Reviewed-on: http://gerrit.cloudera.org:8080/11495 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/clock/hybrid_clock.cc 1 file changed, 16 insertions(+), 16 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iecbb077b1f88293dcb0ab53e40ff3862f772694e Gerrit-Change-Number: 11495 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] hybrid clock: restore SleepFor in WaitUntilAfterLocally
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/11495 ) Change subject: hybrid_clock: restore SleepFor in WaitUntilAfterLocally .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecbb077b1f88293dcb0ab53e40ff3862f772694e Gerrit-Change-Number: 11495 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 24 Sep 2018 15:03:37 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck checksums: Factor out of main ksck code
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11488 ) Change subject: [tools] ksck checksums: Factor out of main ksck code .. Patch Set 6: Verified+1 Precommit failure due to known + unrelated bug KUDU-1678. Overriding. -- To view, visit http://gerrit.cloudera.org:8080/11488 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bb1f51af22ab0c6c20b9426dbb62ea48413ed5b Gerrit-Change-Number: 11488 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 14:36:46 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck checksums: Factor out of main ksck code
Will Berkeley has removed a vote on this change. Change subject: [tools] ksck checksums: Factor out of main ksck code .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11488 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I4bb1f51af22ab0c6c20b9426dbb62ea48413ed5b Gerrit-Change-Number: 11488 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] ksck checksums: Factor out of main ksck code
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11488 ) Change subject: [tools] ksck checksums: Factor out of main ksck code .. Patch Set 5: Fixed some bad indenting. -- To view, visit http://gerrit.cloudera.org:8080/11488 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bb1f51af22ab0c6c20b9426dbb62ea48413ed5b Gerrit-Change-Number: 11488 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 09:34:46 + Gerrit-HasComments: No
[kudu-CR] [tools] ksck checksums: Factor out of main ksck code
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong, Grant Henke, Todd
Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11488
to look at the new patch set (#6).
Change subject: [tools] ksck checksums: Factor out of main ksck code
..
[tools] ksck checksums: Factor out of main ksck code
To prepare to address KUDU-2179 and to improve the organization of the
code, this patch factors out checksum-related classes and code out of
ksck.{cc,h} into a new pair of files ksck_checksum.{cc,h}. Following
the pattern of other ksck-related classes, it also renames some classes
so they start with "Ksck".
There are no functional changes in this patch.
Change-Id: I4bb1f51af22ab0c6c20b9426dbb62ea48413ed5b
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/cluster_verifier.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
A src/kudu/tools/ksck_checksum.cc
A src/kudu/tools/ksck_checksum.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
11 files changed, 356 insertions(+), 236 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11488/6
--
To view, visit http://gerrit.cloudera.org:8080/11488
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bb1f51af22ab0c6c20b9426dbb62ea48413ed5b
Gerrit-Change-Number: 11488
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] ksck checksums: Add KsckChecksummer class
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11498 Change subject: [tools] ksck checksums: Add KsckChecksummer class .. [tools] ksck checksums: Add KsckChecksummer class This removes the remaining checksum logic out of Ksck and into a new KsckChecksummer class, along with refactoring some parts of the logic into separate functions. There are no functional changes. The logic that was not refactored is directly related to KUDU-2179, and will be addressed in a follow-up patch. Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa --- M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_checksum.cc M src/kudu/tools/ksck_checksum.h 4 files changed, 333 insertions(+), 184 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/11498/1 -- To view, visit http://gerrit.cloudera.org:8080/11498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa Gerrit-Change-Number: 11498 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] Implement BloomFilter Predicate in server side.
ZhangYao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11100 )
Change subject: Implement BloomFilter Predicate in server side.
..
Patch Set 8:
(66 comments)
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@45
PS6, Line 45: public:
> Can you use SeedRandom() instead? That way we'll use a different seed for e
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@77
PS6, Line 77: Random rand(SeedRandom());
: uint64_t current = 0;
> Nit: BloomFilterBuilder* bfb1
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@79
PS6, Line 79: for (int i = 0; i < 2; ++i) {
> Don't need if you use SeedRandom().
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@83
PS6, Line 83: continue;
> Use util/Random instead.
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@88
PS6, Line 88:
> const uint8_t*
Ok, got it, thanks a lot. Done.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@92
PS6, Line 92: }
> emplace_back in new code. Elsewhere too.
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@791
PS6, Line 791: vector orig_bloom_filters
= *bf;
> Don't need std:: prefix. Below too.
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@793
PS6, Line 793: // NONE
> Nit: maybe name "orig_bloom_filters" to better convey its purpose?
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@828
PS6, Line 828: ColumnPredicate::IsNotNull(column),
> bf_copy maybe?
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@1382
PS6, Line 1382: BloomFilterBuilder bfb1(
> 0 1 both hit
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@1414
PS6, Line 1414: vector keys_slice;
> 0 1 both hit
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate-test.cc@1429
PS6, Line 1429: BloomFilterSizing::ByCountAndFPRate(n_keys, 0.01));
> Each of these "Test for ... type" is independent of the other tests, right?
Actually it is not independent. This Test is for InBloomFilter merge and the
test is formed by three types tests:uint64, string and binary just like the
other predicate test, so I think they should be together.
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@275
PS6, Line 275:
: BloomFilterInner() : nhash_(0), hash_algorithm_(CITY_HASH) {}
:
: const Slice& bloom_data() const {
: return bloom_data_;
: }
:
: size_t nhash() const {
: return nhash_;
: }
:
: HashAlgorithmInBloomFilter hash_algorithm() const {
: return hash_algorithm_;
: }
:
: void set_nhash(size_t nhash) {
: nhash_ = nhash;
: }
:
: void set_bloom_data(Slice bloom_data) {
: bloom_data_ = bloom_data;
: }
:
: v
> I don't see the point of these getters and setters since the members are al
Emmm I simply try to use setters and getters to let the calls look uniform.
Maybe I should change the struct to class. Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@301
PS6, Line 301:
: bool operator==(const BloomFilterInner& other) const {
> Nit: reformat like:
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@304
PS6, Line 304: nhash_ == other.nhash() &&
> Nit: separate each member from the member before it (or from operator==) wi
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@308
PS6, Line 308:
:
> I don't think this comment is necessary; the list of supported algorithms i
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@354
PS6, Line 354: into this IS NOT NULL
> Nit: const ColumnPredicate&
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.h@425
PS6, Line 425:
> Nit: "The list of bloom filters in this predicate."
Done
http://gerrit.cloudera.org:8080/#/c/11100/6/src/kudu/common/column_predicate.cc
File src/
[kudu-CR] KUDU-2245 Graceful leadership transfer
Hello Fengling Wang, Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11251 to look at the new patch set (#11). Change subject: KUDU-2245 Graceful leadership transfer .. KUDU-2245 Graceful leadership transfer This patch implements graceful leadership transfer, as described in the original Raft thesis. It has the following steps: 1. An admin client sends a request to the tablet leader for it to transfer leadership. The client can indicate a specific voter that it wants to become the leader, or it can allow the current leader to choose its successor. 2. The leader receives the request and beings a leader transfer period. During a leader transfer period, the leader does not accept writes or config change requests. This allows followers to catch up to the leader. A background timer expires the transfer period after one election timeout, since clients should be able to ride over interruptions in service lasting at least that long. 3. During the transfer period, the leader continues to update peers. When it receives a response from a peer, it checks if that peer is a voter and fully caught up to the leader's log. If it is, and if it is the designated successor if one was provided, the leader signals the peer to start an election, which it should win. If no eligible successor appears, the transfer period expires and the leader resumes normal operation. This is an improvement over the current leader step down method, which causes the leader to simply relinquish leadership and snooze its election timer for an extra long period, so another voter will likely become leader. Leadership transfer should usually be much faster and it allows the client to select the new leader among current voters. However, note that it does not provide strictly better guarantees- it is still possible that leadership will not be transferred. I ran TestRepeatLeaderStepDown and TestGracefulLeaderStepDown 1000 times and 200 times each, in debug and TSAN modes, with 4 stress threads, and saw no failures. Still WIP because I want to * Run some dist-test loops of the rebalancer tests, which now use graceful leadership transfer. * Add a test or two for bad cases where the leadership transfer period should expire. * Quantify how much faster leadership transfer is than abrupt stepdown, at least in a lab environment. Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_peers.h M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/peer_manager.cc M src/kudu/consensus/peer_manager.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h M src/kudu/tserver/tablet_service.cc 17 files changed, 859 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/11251/11 -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 11 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2245 Graceful leadership transfer
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11251 ) Change subject: KUDU-2245 Graceful leadership transfer .. Patch Set 10: (9 comments) I tried doing a bit of a refactor on the CheckMoveComplete code but maybe I do not understand it as well as I thought. Maybe we should chat about it tomorrow? http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc File src/kudu/tools/tool_replica_util.cc: http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a198 PS10, Line 198: 0. 'from_ts_uuid' is X (in the failure logs, X = 51a2892fa5c1471fb173f2dda0d7d11a. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a206 PS10, Line 206: : : : : 1. We find out the tablet leader and build a proxy to it. In the failure case, 'from_ts_uuid' equals 'leader_uuid' equals X. 'proxy' is an RPC proxy to X. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a259 PS10, Line 259: : : : : : : 3. We do a graceful stepdown of TS with UUID 'leader_uuid', which is X. Note that 'proxy' continues to point to X. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a282 PS10, Line 282: : : 4. We re-find the current leader and store it in 'leader_uuid', shadowing 'leader_uuid'. If leadership has already changed and client detects this, 'leader_uuid' is not X anymore. It's now, say, Y. Note that 'proxy' is still a proxy to X. In the failure logs, Y = d56040e17c444e06b1924246d25f1263. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a285 PS10, Line 285: 5. This evaluates to true, since 'from_ts_uuid' is X, the original leader, but now 'leader_uuid' is Y. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@a290 PS10, Line 290: 6. Uh oh. 'proxy' is a proxy to X, but 'leader_uuid', which determines the destination UUID of the GetConsensus RPC, is Y. Thus the RPC fails with Invalid argument: GetConsensusState: Wrong destination UUID requested. Local UUID: X. Requested UUID: Y http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@296 PS10, Line 296: // We should consult the leader for the most up-to-date consensus state. : string new_leader_uuid; : HostPort new_leader_hp; : RETURN_NOT_OK(GetTabletLeader(client, tablet_id, &new_leader_uuid, &new_leader_hp)); > I think this is racy, regardless of the way how leadership changes -- grace Right, we rely on retries by the caller to handle that sort of thing. 313 is downstream of this leader determination and can't replace it, as I understand the code. We have to find the new leader else we won't know if we've successfully moved leadership from 'from_ts_uuid' (in the case where that's necessary). http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@305 PS10, Line 305: leader_uuid > If leader changed and new leader UUID is detected, why still to keep this o This is a mistake. We want to check if the current leader's uuid after step down is the same as the 'from_ts_uuid'. Thanks for finding it. http://gerrit.cloudera.org:8080/#/c/11251/10/src/kudu/tools/tool_replica_util.cc@310 PS10, Line 310: proxy > I think this code is executed when no DoLeaderStepDown() was called above ( Maybe some naming cleanup or a bit of refactoring would help? 'from_ts_uuid != leader_uuid' on L305 is saying that (in the case when the leadership started on 'from_ts_uuid') leadership has changed to some other peer. I added some numbered comments to the original code to better explain what I think is going on, and I'll make an attempt to make it clearer what's going on here, at least as I understand it. -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 10 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 24 Sep 2018 08:18:42 + Gerrit-HasComments: Yes
[kudu-CR] Implement BloomFilter Predicate in server side.
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11100 to look at the new patch set (#8). Change subject: Implement BloomFilter Predicate in server side. .. Implement BloomFilter Predicate in server side. Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c --- M src/kudu/common/column_predicate-test.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h M src/kudu/common/common.proto M src/kudu/common/key_util.cc M src/kudu/common/scan_spec.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc M src/kudu/tablet/cfile_set-test.cc M src/kudu/util/bloom_filter.h 10 files changed, 1,116 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/11100/8 -- To view, visit http://gerrit.cloudera.org:8080/11100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c Gerrit-Change-Number: 11100 Gerrit-PatchSet: 8 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: ZhangYao
