[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Alexey Serbin has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 2:
(5 comments)
Added some after-thought nits: they are about making sure my understanding was
correct.
http://gerrit.cloudera.org:8080/#/c/7888/2//COMMIT_MSG
Commit Message:
Line 7: consensus: Save previous last-logged OpId across tablet copies
nit: as an afterthought, I it would be nice to add some more information on how
the system behave prior to this patch in the particular case which is now
exercised by the test. Is that something like 'IllegalState: must be running
to vote when last-logged opid is not known' or it was something more cryptic?
Would it make sense?
http://gerrit.cloudera.org:8080/#/c/7888/2/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS2, Line 2846: TEST_F(RaftConsensusITest,
TestTombstonedVoteAfterFailedTabletCopy)
Just for my better understanding: did this test fail before the fix?
PS2, Line 2849: vector ts_flags = {
: // We write 128KB cells in this test, so bump the limit.
: "--max_cell_size_bytes=100"
: };
nit: could you add a comment explaining why it's crucial to write above the
default cell size limit?
Line 2860: ASSERT_EQ(3, active_tablet_servers.size());
nit: Since the FLAGS_num_replicas might be altered in the command line, would
it make sense to add:
ASSERT_EQ(3, FLAGS_num_replicas);
?
PS2, Line 2893:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
nit (just wanted to make sure my understanding is correct): would it work if
bringing back not the former leader, but the other replica (not the crashed
follower, of course)?
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Adar Dembo has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 2: (1 comment) Just passing through... http://gerrit.cloudera.org:8080/#/c/7888/2//COMMIT_MSG Commit Message: Line 7: consensus: Save previous last-logged OpId across tablet copies > nit: as an afterthought, I it would be nice to add some more information on Could you flesh out the commit description? Why is saving the last-logged OpId a good thing (or the right thing) to do? -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Hello David Ribeiro Alves, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7904 to review the following change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, If a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged opid field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged opid of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 84 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/7904/1 -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Hello Dan Burkert, David Ribeiro Alves, Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7905 to review the following change. Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/7905/1 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Mike Percy
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Mike Percy has uploaded a new patch set (#2). Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, If a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged opid field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged opid of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/7904/2 -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Adar Dembo has uploaded a new patch set (#2). Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/7905/2 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7905 to look at the new patch set (#3). Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/7905/3 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Jean-Daniel Cryans has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7905/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 121: has* been? -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Alexey Serbin has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7905/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 121: been has been? -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Alexey Serbin has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7905/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 126: Other improvements nit: would nit make sense to elaborate a bit on this or it's not worth mentioning in the release notes? -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Adar Dembo has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/7905/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 121: been > has been? Done PS3, Line 121: > has* been? Done PS3, Line 126: Other improvements > nit: would nit make sense to elaborate a bit on this or it's not worth ment This stuff includes going from per-replica thread pools to a shared but unbounded thread pool, but the nuance is hard to capture in a sentence or two, so I don't think it's worth including. -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7905 to look at the new patch set (#4). Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/7905/4 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7824 to look at the new patch set (#4). Change subject: java-client: improve error messages when failing to connect to secure cluster .. java-client: improve error messages when failing to connect to secure cluster Cleans up the error handling in Connection.java so that is passes through non-recoverable exceptions, and improves the error which results from attempting to connect to a secure cluster with an unauthenticated client. Example of the new error: org.apache.kudu.client.NonRecoverableException: Couldn't find a valid master in (nightly512-1.gce.cloudera.com:7051). Exceptions received: org.apache.kudu.client.NonRecoverableException: Server requires Kerberos, but this client is not authenticated (kinit) Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java 4 files changed, 108 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/7824/4 -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Jean-Daniel Cryans has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Dan Burkert has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) docs: add 1.5.0 release note for thread count reduction
Dan Burkert has posted comments on this change. Change subject: docs: add 1.5.0 release note for thread count reduction .. Patch Set 1: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-HasComments: No
[kudu-CR] docs: add 1.5.0 release note for thread count reduction
Dan Burkert has submitted this change and it was merged. Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Reviewed-on: http://gerrit.cloudera.org:8080/7905 Reviewed-by: Jean-Daniel Cryans Tested-by: Dan Burkert --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) Approvals: Dan Burkert: Verified Jean-Daniel Cryans: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7905 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR](branch-1.5.x) docs: add 1.5.0 release note for thread count reduction
Dan Burkert has uploaded a new change for review. http://gerrit.cloudera.org:8080/7906 Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Reviewed-on: http://gerrit.cloudera.org:8080/7905 Reviewed-by: Jean-Daniel Cryans Tested-by: Dan Burkert (cherry picked from commit 595af7cdfb6b5e3c2d8adcd74ffb97032bae4ff3) --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/7906/1 -- To view, visit http://gerrit.cloudera.org:8080/7906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo
[kudu-CR](branch-1.5.x) docs: add 1.5.0 release note for thread count reduction
Dan Burkert has submitted this change and it was merged. Change subject: docs: add 1.5.0 release note for thread count reduction .. docs: add 1.5.0 release note for thread count reduction Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Reviewed-on: http://gerrit.cloudera.org:8080/7905 Reviewed-by: Jean-Daniel Cryans Tested-by: Dan Burkert (cherry picked from commit 595af7cdfb6b5e3c2d8adcd74ffb97032bae4ff3) Reviewed-on: http://gerrit.cloudera.org:8080/7906 Reviewed-by: Dan Burkert --- M docs/release_notes.adoc 1 file changed, 11 insertions(+), 0 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/7906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I06939f0e01db780f86a7da16e92e5acd188b6925 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1811: C++ client: use larger batches when fetching scan tokens
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens .. KUDU-1811: C++ client: use larger batches when fetching scan tokens Improve C++ client scan token generation performance by fetching larger batches. Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Reviewed-on: http://gerrit.cloudera.org:8080/7748 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc 3 files changed, 31 insertions(+), 14 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jun He Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jun He Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1811: C++ client: use larger batches when fetching scan tokens
Dan Burkert has posted comments on this change. Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jun He Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jun He Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) KUDU-1811: C++ client: use larger batches when fetching scan tokens
Dan Burkert has uploaded a new change for review. http://gerrit.cloudera.org:8080/7907 Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens .. KUDU-1811: C++ client: use larger batches when fetching scan tokens Improve C++ client scan token generation performance by fetching larger batches. Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Reviewed-on: http://gerrit.cloudera.org:8080/7748 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert (cherry picked from commit 738f345a19047ae07bbcc9704f2243f9e9c8bd40) --- M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc 3 files changed, 31 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/7907/1 -- To view, visit http://gerrit.cloudera.org:8080/7907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Jun He
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Alexey Serbin has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/7904/2//COMMIT_MSG Commit Message: PS2, Line 7: A nit: remove the capitalization of the 'a' letter PS2, Line 13: If nit: if PS2, Line 20: OpId nit: opid I mean, consider using the same style for that: either 'opid' or 'OpId' http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: PS2, Line 1142: int new_replica_idx = cluster_->tablet_server_index_by_uuid(new_replica_uuid); just to clarify: it's guaranteed that the new tablet replica will land at the newly added server, right? PS2, Line 1148: }); How do we know that the tablet will not start running if the ASSERT_EVENTUALLY above has been satisfied with TABLET_DATA_COPYING state? PS2, Line 1156: cluster_->tablet_server(i)->mutable_flags()->pop_back(); paranoid nit: since the arrangement of the flags passed to StartCluster() in the mutable_flags() of the tablet server is an implementation detail, would it make sense to add an assert on the last element to be tablet_copy_early_session_timeout_prob indeed? http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS2, Line 316: MakeOpId(1, 0); I noticed there is MinimumOpId() method in opid_util.h. Why not to use that instead of MakeOpId(1, 0)? -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Alexey Serbin has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: PS2, Line 1148: }); > How do we know that the tablet will not start running if the ASSERT_EVENTUA Ah, that's because of tablet_copy_early_session_timeout_prob=1.0: it should end the copy anyway (unless it could not find the corresponding session) -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](branch-1.5.x) KUDU-1811: C++ client: use larger batches when fetching scan tokens
Dan Burkert has posted comments on this change. Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jun He Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) KUDU-1811: C++ client: use larger batches when fetching scan tokens
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens .. KUDU-1811: C++ client: use larger batches when fetching scan tokens Improve C++ client scan token generation performance by fetching larger batches. Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Reviewed-on: http://gerrit.cloudera.org:8080/7748 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert (cherry picked from commit 738f345a19047ae07bbcc9704f2243f9e9c8bd40) Reviewed-on: http://gerrit.cloudera.org:8080/7907 --- M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc 3 files changed, 31 insertions(+), 14 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jun He Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Alexey Serbin has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 4: (1 comment) LGTM, just a tiny nit on the line length. http://gerrit.cloudera.org:8080/#/c/7824/4/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java: PS4, Line 684: NonRecoverableExcep nit: this line's length is more than 100 characters -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Alexey Serbin has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7824 to look at the new patch set (#5). Change subject: java-client: improve error messages when failing to connect to secure cluster .. java-client: improve error messages when failing to connect to secure cluster Cleans up the error handling in Connection.java so that is passes through non-recoverable exceptions, and improves the error which results from attempting to connect to a secure cluster with an unauthenticated client. Example of the new error: org.apache.kudu.client.NonRecoverableException: Couldn't find a valid master in (nightly512-1.gce.cloudera.com:7051). Exceptions received: org.apache.kudu.client.NonRecoverableException: Server requires Kerberos, but this client is not authenticated (kinit) Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java 4 files changed, 109 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/7824/5 -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Alexey Serbin has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Dan Burkert has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using
InterruptibleIterator
..
Patch Set 2:
What I had in mind was something like this:
override def hasNext: Boolean = {
while ((currentIterator != null && !currentIterator.hasNext &&
scanner.hasMoreRows) ||
(scanner.hasMoreRows && currentIterator == null)) {
if (TaskContext.get().isInterrupted()) {
throw new RuntimeException("KuduRDD task interrupted")
}
currentIterator = scanner.nextRows()
}
currentIterator.hasNext
}
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: caiconghui
Gerrit-HasComments: No
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Mike Percy has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/7904/2//COMMIT_MSG Commit Message: PS2, Line 7: A > nit: remove the capitalization of the 'a' letter Hrm. This is easy to change but I'm not sold on it and I think this type of comment is generally a little distracting. If we want to standardize on git commit style then we should discuss it on dev@ and reach a consensus. Then, document it. It seems somewhat counterproductive to try to enforce undocumented style standards. On the other hand, maybe I missed a discussion that happened on the list while I wasn't paying attention. PS2, Line 13: If > nit: if Done PS2, Line 20: OpId > nit: opid Well, it should be OpId. Fixed http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: PS2, Line 1142: int new_replica_idx = cluster_->tablet_server_index_by_uuid(new_replica_uuid); > just to clarify: it's guaranteed that the new tablet replica will land at t Yes, because we are explicitly calling AddServer() to do that. PS2, Line 1148: }); > Ah, that's because of tablet_copy_early_session_timeout_prob=1.0: it should correct PS2, Line 1156: cluster_->tablet_server(i)->mutable_flags()->pop_back(); > paranoid nit: since the arrangement of the flags passed to StartCluster() i This isn't the first test to use this idiom. If that implementation was changed, a bunch of tests would break, including this one. http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS2, Line 316: MakeOpId(1, 0); > I noticed there is MinimumOpId() method in opid_util.h. Why not to use tha MinimumOpid() is defined as (0,0) and that will not work because we consider it the same as "empty" in the TabletMetadata code, for historical reasons. If you're OK with it, I'll file a jira to follow up with a cleanup patch that switches the naming around. -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7904 to look at the new patch set (#3). Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, if a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged OpId field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged OpId of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/7904/3 -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Alexey Serbin has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 3: Code-Review+2 (4 comments) http://gerrit.cloudera.org:8080/#/c/7904/2//COMMIT_MSG Commit Message: PS2, Line 7: A > Hrm. This is easy to change but I'm not sold on it and I think this type of It was just a nit, feel free to ignore. I don't think it's worth to enforce some guidelines here, just thought it was a typo. http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: PS2, Line 1142: int new_replica_idx = cluster_->tablet_server_index_by_uuid(new_replica_uuid); > Yes, because we are explicitly calling AddServer() to do that. ok, thanks. PS2, Line 1156: cluster_->tablet_server(i)->mutable_flags()->pop_back(); > This isn't the first test to use this idiom. If that implementation was cha SGTM. http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS2, Line 316: MakeOpId(1, 0); > MinimumOpid() is defined as (0,0) and that will not work because we conside SGTM -- I just wanted to understand why MinimumOpId() would not fit here. -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7888/2//COMMIT_MSG
Commit Message:
Line 7: consensus: Save previous last-logged OpId across tablet copies
> Could you flesh out the commit description? Why is saving the last-logged O
Done; Done
http://gerrit.cloudera.org:8080/#/c/7888/2/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS2, Line 2846: TEST_F(RaftConsensusITest,
TestTombstonedVoteAfterFailedTabletCopy)
> Just for my better understanding: did this test fail before the fix?
Yes, this test failed before the fix.
PS2, Line 2849: vector ts_flags = {
: // We write 128KB cells in this test, so bump the limit.
: "--max_cell_size_bytes=100"
: };
> nit: could you add a comment explaining why it's crucial to write above the
I just moved this to CauseFollowerToFallBehindLogGC()
Line 2860: ASSERT_EQ(3, active_tablet_servers.size());
> nit: Since the FLAGS_num_replicas might be altered in the command line, wou
Done
PS2, Line 2893:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
> nit (just wanted to make sure my understanding is correct): would it work i
Yes, we should be able to bring back any other two, since that was a majority.
However I already had a handle to these two, and it was simpler to write than
looping and trying to make sure we revived the tombstoned one and two (but not
all 3) of the others.
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7888 to look at the new patch set (#3). Change subject: consensus: Save previous last-logged OpId across tablet copies .. consensus: Save previous last-logged OpId across tablet copies When a replica falls behind the leader's log, and then fails a tablet copy, it should still be able to vote in leader elections while tombstoned. With this patch, that becomes possible. This helps with availability. Without this patch, asking a poat-tablet copy failure replica to vote would result in the following RPC response: IllegalState: must be running to vote when last-logged opid is not known Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 5 files changed, 81 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7888/3 -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Adar Dembo has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7888/3//COMMIT_MSG Commit Message: PS3, Line 14: poat Thanks for adding the description. There's a typo here. -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Mike Percy has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7904/2/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS2, Line 316: MakeOpId(1, 0); > SGTM -- I just wanted to understand why MinimumOpId() would not fit here. I filed KUDU-2122 and assigned it to myself. -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7888/3//COMMIT_MSG Commit Message: PS3, Line 14: poat > Thanks for adding the description. There's a typo here. erg -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7888 to look at the new patch set (#4). Change subject: consensus: Save previous last-logged OpId across tablet copies .. consensus: Save previous last-logged OpId across tablet copies When a replica falls behind the leader's log, and then fails a tablet copy, it should still be able to vote in leader elections while tombstoned. With this patch, that becomes possible. This helps with availability. Without this patch, asking a post-tablet copy failure replica to vote would result in the following RPC response: IllegalState: must be running to vote when last-logged opid is not known Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 5 files changed, 81 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7888/4 -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Hello Dan Burkert, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/7909
to review the following change.
Change subject: master: always use smart pointers when accessing TableInfo and
TabletInfo
..
master: always use smart pointers when accessing TableInfo and TabletInfo
Mixing scoped_refptr and raw access is error-prone and complicated code by
forcing the use of both in certain places. It's fair to be concerned about
the overhead; as far as I can tell it's minimal, because it's either in
functions that are already "slow" (like {Create,Alter,Delete}Table and the
equivalent in CatalogManagerBgTasks), or the call sites pass the object by
cref and thus avoid an extra ref/deref in the first place.
I also reduced the number of ways to add/remove TabletInfo objects to a
TableInfo down to one.
Finally, I couldn't help myself and did some more modernization:
- Removed a couple std:: prefixes.
- Added "auto" to some loops.
- Replaced some vector::push_back calls with emplace_back.
- Reformatted double '>' (C++11 allows them to be adjacent).
Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
---
M src/kudu/master/catalog_manager-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 123 insertions(+), 182 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/7909/1
--
To view, visit http://gerrit.cloudera.org:8080/7909
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
[kudu-CR] KUDU-2078: Sink failure if batch size > session's flush buffer size
Mike Percy has posted comments on this change. Change subject: KUDU-2078: Sink failure if batch size > session's flush buffer size .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id1c54bcecc3e13ae64dd90efe6cf53021517dcdf Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-2078: Sink failure if batch size > session's flush buffer size
Mike Percy has posted comments on this change. Change subject: KUDU-2078: Sink failure if batch size > session's flush buffer size .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7641/1/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java File java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java: PS1, Line 70: 100 doc needs updating -- To view, visit http://gerrit.cloudera.org:8080/7641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id1c54bcecc3e13ae64dd90efe6cf53021517dcdf Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Adar Dembo has uploaded a new patch set (#2).
Change subject: master: always use smart pointers when accessing TableInfo and
TabletInfo
..
master: always use smart pointers when accessing TableInfo and TabletInfo
Mixing scoped_refptr and raw access is error-prone and complicated code by
forcing the use of both in certain places. It's fair to be concerned about
the overhead; as far as I can tell it's minimal, because it's either in
functions that are already "slow" (like {Create,Alter,Delete}Table and the
equivalent in CatalogManagerBgTasks), or the call sites pass the object by
cref and thus avoid an extra ref/deref in the first place.
I also reduced the number of ways to add/remove TabletInfo objects to a
TableInfo down to one.
Finally, I couldn't help myself and did some more modernization:
- Removed a couple std:: prefixes.
- Added "auto" to some loops.
- Replaced some vector::push_back calls with emplace_back.
- Reformatted double '>' (C++11 allows them to be adjacent).
Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
---
M src/kudu/master/catalog_manager-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 161 insertions(+), 231 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/7909/2
--
To view, visit http://gerrit.cloudera.org:8080/7909
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
caiconghui has posted comments on this change.
Change subject: feat: add the wrapper for the Iterator by using
InterruptibleIterator
..
Patch Set 2:
ï¼ Dan Burkert　Yeah, I think throw TaskKilledException("KuduRDD task
interrupted") is more accurate, that is also what InterruptibleIterator throw
in the latest spark version, and you said writing a test, but I cannot find a
public api to kill task, so I haven't push the new patch yet
--
To view, visit http://gerrit.cloudera.org:8080/7753
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: caiconghui
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: caiconghui
Gerrit-HasComments: No
[kudu-CR] KUDU-1755 Part 2: Improve tablet on-disk size metric
Mike Percy has posted comments on this change.
Change subject: KUDU-1755 Part 2: Improve tablet on-disk size metric
..
Patch Set 10:
(10 comments)
Would be nice to add a simple test for the values stored by the new metrics.
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/consensus/consensus_meta.h
File src/kudu/consensus/consensus_meta.h:
Line 153: return on_disk_size_;
Can we say this is thread-safe and use an atomic?
PS10, Line 241: uint64_t
how about atomic and int64_t because there is just no way we are going to need
that 64th bit, plus GSG says to prefer signed integers when possible
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:
PS10, Line 922: CHECK_OK
Do we need this CHECK? Also, can we make this safe to run even if log_ is shut
down?
Some reasonable options:
1. return Status
2. return 0 if not open
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/consensus/log.h
File src/kudu/consensus/log.h:
PS10, Line 201: TotalSize
maybe name this OnDiskSize() for consistency?
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
Line 2645: UniqueLock lock(lock_);
If we make cmeta_->on_disk_size() thread-safe, then we can avoid taking the
consensus lock here. Additional note on why this would be safe: cmeta_ is set
only once in Init(), and RaftConsensus::Create() requires Init() to succeed in
order to publish the RaftConsensus instance, so we can generally treat the
cmeta_ reference as a const.
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:
Line 690: shared_lock l(component_lock_);
1. Can we just take a ref to base_data_ instead of holding the component_lock_
while calling base_data_->OnDiskSize() ? I'm not 100% sure, but it looks like
it.
2. Do we need to hold component_lock_ while calling
delta_tracker_->OnDiskSize()? I don't see a reason for that, since (a) as long
as the DRS is open_, the delta_tracker_ reference is safe to access without a
lock because it's set-once and (b) DeltaTracker::OnDiskSize() is thread-safe.
http://gerrit.cloudera.org:8080/#/c/6968/10/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:
PS10, Line 377: OnDiskSizeNoCMetaUnlocked
why the "no cmeta" version? could use a comment
PS10, Line 645: auto consensus = consensus_;
: if (consensus) {
because consensus_ is set-once, this can be written as:
if (consensus_) {
ret += consensus_->MetadataOnDiskSize();
}
PS10, Line 656: size_t
Can we use int64_t instead of size_t? We are never going to have 64-bit length
data on a single node.
See the GSG on unsigned types in general:
https://google.github.io/styleguide/cppguide.html#Integer_Types
The exceptions to that rule are when storing things like pointers, or when
wrapping an existing API makes it basically impossible.
PS10, Line 660: if (tablet_) {
: ret += tablet_->OnDiskSize();
: }
: // WAL segments.
: if (state_ == RUNNING) {
: ret += log_->TotalSize();
: }
Instead of doing this work while holding TabletReplica::lock_, can this be
structured so we can just grabs the refs from TabletReplica and then release
the lock?
Something like:
int64_t size = 0;
scoped_refptr tablet;
scoped_refptr log;
{
lock_guard l(lock_);
tablet = tablet_;
log = log_;
}
if (tablet) {
size += tablet->OnDiskSize();
}
if (log) {
size += log->OnDiskSize();
}
return ret;
--
To view, visit http://gerrit.cloudera.org:8080/6968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia83f8aae0a544abbd19f5a34520b4828b89b6315
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
David Ribeiro Alves has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7888/4/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS4, Line 2884: int follower_idx =
cluster_->tablet_server_index_by_uuid(follower_uuid);
: ASSERT_OK(inspect_->CheckTabletDataStateOnTS(follower_idx,
tablet_id_, { TABLET_DATA_COPYING }));
:
: ASSERT_OK(cluster_->master()->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(follower_uuid)->Restart());
is there a way to assert that the follower doesn't have a last op id?
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Alexey Serbin has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 4: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/7888/2/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: PS2, Line 2846: vector ts_flags; > Yes, this test failed before the fix. ok, great! PS2, Line 2893: workload.set_timeout_allowed(true); > Yes, we should be able to bring back any other two, since that was a majori Thank you for the clarification. -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7888/4/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS4, Line 2884: int follower_idx =
cluster_->tablet_server_index_by_uuid(follower_uuid);
: ASSERT_OK(inspect_->CheckTabletDataStateOnTS(follower_idx,
tablet_id_, { TABLET_DATA_COPYING }));
:
: ASSERT_OK(cluster_->master()->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(follower_uuid)->Restart());
> is there a way to assert that the follower doesn't have a last op id?
It will in this case, and we do assert on it functionally, because if it didn't
then the new leader would not get elected and be able to tablet copy over the
follower, and we would not be able to write to the tablet again below.
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7888/2/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: PS2, Line 2893: ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart()); > Thank you for the clarification. Err, I slightly messed up my explanation (I was thinking about another test I recently wrote). I should have said: it was simpler to write than looping and trying to make sure we revived the tombstoned one (the follower) and one (but not both) of the others. -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
David Ribeiro Alves has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7888/4/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS4, Line 2884: int follower_idx =
cluster_->tablet_server_index_by_uuid(follower_uuid);
: ASSERT_OK(inspect_->CheckTabletDataStateOnTS(follower_idx,
tablet_id_, { TABLET_DATA_COPYING }));
:
: ASSERT_OK(cluster_->master()->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(follower_uuid)->Restart());
> It will in this case, and we do assert on it functionally, because if it di
hum, but that would happen also if there was a last op id, right? (this is the
"additional" case you're solving here, or am I missing something)
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
David Ribeiro Alves has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add tablet state summary metrics and fix KUDU-2044
Mike Percy has posted comments on this change.
Change subject: Add tablet state summary metrics and fix KUDU-2044
..
Patch Set 4:
(1 comment)
Overall the new metrics look reasonable. But it seems like this should be
written as 2 separate patches, because I'm not sure about the use case for
hiding the metric entity per my comment in tablet.cc
http://gerrit.cloudera.org:8080/#/c/7618/4/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
Line 277: if (metric_entity_) {
Instead of hiding the metric entity, if that is the effect we want, shouldn't
we instead destroy and unregister the tablet's metric entity on Shutdown()?
--
To view, visit http://gerrit.cloudera.org:8080/7618
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c82987ffe4f37e8af95972bde97841e44c521d9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has posted comments on this change.
Change subject: consensus: Save previous last-logged OpId across tablet copies
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7888/4/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:
PS4, Line 2884: int follower_idx =
cluster_->tablet_server_index_by_uuid(follower_uuid);
: ASSERT_OK(inspect_->CheckTabletDataStateOnTS(follower_idx,
tablet_id_, { TABLET_DATA_COPYING }));
:
: ASSERT_OK(cluster_->master()->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(leader_uuid)->Restart());
:
ASSERT_OK(cluster_->tablet_server_by_uuid(follower_uuid)->Restart());
> hum, but that would happen also if there was a last op id, right? (this is
Just reiterating what we discussed over chat: since we're now clobbering the
last-logged opid in one fewer instance, we're able to successfully vote in that
case, which serves as a functional regression test.
--
To view, visit http://gerrit.cloudera.org:8080/7888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: David Ribeiro Alves
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] consensus: Save previous last-logged OpId across tablet copies
Mike Percy has submitted this change and it was merged. Change subject: consensus: Save previous last-logged OpId across tablet copies .. consensus: Save previous last-logged OpId across tablet copies When a replica falls behind the leader's log, and then fails a tablet copy, it should still be able to vote in leader elections while tombstoned. With this patch, that becomes possible. This helps with availability. Without this patch, asking a post-tablet copy failure replica to vote would result in the following RPC response: IllegalState: must be running to vote when last-logged opid is not known Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Reviewed-on: http://gerrit.cloudera.org:8080/7888 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: David Ribeiro Alves --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 5 files changed, 81 insertions(+), 13 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] tablet copy: Allow voting from failed initial tablet copies
Mike Percy has submitted this change and it was merged. Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, if a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged OpId field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged OpId of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Reviewed-on: http://gerrit.cloudera.org:8080/7904 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 85 insertions(+), 0 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7904 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR](branch-1.5.x) KUDU-2118. Fully shut down TabletReplica on delete
Hello Dan Burkert, Adar Dembo, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7911 to review the following change. Change subject: KUDU-2118. Fully shut down TabletReplica on delete .. KUDU-2118. Fully shut down TabletReplica on delete After the various lifecycle changes implemented in 5bca7d8ba185d62952fb3e3163cbe88d20453da0 we were left with a case where we fail to fully shut down TABLET_DATA_DELETED replicas before dereferencing them. Because LeaderElection via ElectionDecisionCallback will hold a reference to RaftConsensus while a vote is taking place, the leader election callback running on the reactor thread may destroy the RaftConsensus object. If that occurs without RaftConsensus already being in a fully-shutdown state, the RaftConsensus destructor runs Shutdown() before destoying the object, which takes a lock and triggers an assertion via AssertWaitAllowed(). This crashes the server. This patch fixes the issue by calling Shutdown() on TABLET_DATA_DELETED replicas before dereferencing them in TSTabletManager. A concurrency test is included that spawns threads to start elections while the tablets are being deleted. This test pretty reliably fails without the included fix, and is solidly stable with the fix. Change-Id: I4a0ea46f220a10d4de680fc73d45d14426e0395e Reviewed-on: http://gerrit.cloudera.org:8080/7883 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/integration-tests/delete_tablet-itest.cc M src/kudu/tserver/ts_tablet_manager.cc 2 files changed, 59 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/7911/1 -- To view, visit http://gerrit.cloudera.org:8080/7911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4a0ea46f220a10d4de680fc73d45d14426e0395e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) tablet copy: Allow voting from failed initial tablet copies
Hello Dan Burkert, Alexey Serbin, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7913 to review the following change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, if a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged OpId field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged OpId of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Reviewed-on: http://gerrit.cloudera.org:8080/7904 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/7913/1 -- To view, visit http://gerrit.cloudera.org:8080/7913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) consensus: Save previous last-logged OpId across tablet copies
Hello Dan Burkert, David Ribeiro Alves, Alexey Serbin, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7912 to review the following change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. consensus: Save previous last-logged OpId across tablet copies When a replica falls behind the leader's log, and then fails a tablet copy, it should still be able to vote in leader elections while tombstoned. With this patch, that becomes possible. This helps with availability. Without this patch, asking a post-tablet copy failure replica to vote would result in the following RPC response: IllegalState: must be running to vote when last-logged opid is not known Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Reviewed-on: http://gerrit.cloudera.org:8080/7888 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: David Ribeiro Alves --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 5 files changed, 81 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/7912/1 -- To view, visit http://gerrit.cloudera.org:8080/7912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Alexey Serbin has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 2: (4 comments) just some nits. http://gerrit.cloudera.org:8080/#/c/7909/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS2, Line 1788: map> nit: would TabletInfo::TabletInfoMap be a better choice here? PS2, Line 1789: map> ditto PS2, Line 3683: .get() nit: it seems this might be dropped PS2, Line 3684: .get() ditto -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7656 to look at the new patch set (#3). Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks .. KUDU-2055: Coalesce hole punching when deleting groups of blocks Similar to 'BlockCreationTransaction', this patch adds a new layer of abstraction at Block Manager to coalesce blocks deletions in a logical operation, e.g. compaction. By coalescing blocks deletions, the number of hole punching in LBM will be reduced from one per block to one per log block container. This should overall optimize the performance of operation that involves batch deletions, such as compaction and hole repunching. Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 --- M src/kudu/cfile/bloomfile.cc M src/kudu/cfile/bloomfile.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/fs/block_manager-test.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/file_block_manager.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/multi_column_writer.cc M src/kudu/tablet/multi_column_writer.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client-test.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 22 files changed, 329 insertions(+), 150 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/7656/3 -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks
Hao Hao has posted comments on this change. Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks .. Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/7656/2/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: Line 182: > I think the docs on the 'deleted' parameter could use some work here - I'm Done Line 273: // Returned block IDs are not guaranteed to be in any particular order, > Since this is infallible (doesn't return Status), could it return the scope Done http://gerrit.cloudera.org:8080/#/c/7656/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 1184: std::sort(log_blocks.begin(), log_blocks.end(), > Not continue? Done Line 1217: // If we cannot find the block, then the block could be actually > can drop the std:: prefix Done Line 1225: deleted.emplace_back(block); > lowercase first letter Done Line 1983: > consider using CreateDeletionTransaction That would introduce a downcast? PS2, Line 2173: d > lowercase first letter Done -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has uploaded a new change for review. http://gerrit.cloudera.org:8080/7914 Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 31 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/1 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert
[kudu-CR](branch-1.5.x) tablet copy: Allow voting from failed initial tablet copies
Dan Burkert has posted comments on this change. Change subject: tablet copy: Allow voting from failed initial tablet copies .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) KUDU-2118. Fully shut down TabletReplica on delete
Dan Burkert has posted comments on this change. Change subject: KUDU-2118. Fully shut down TabletReplica on delete .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a0ea46f220a10d4de680fc73d45d14426e0395e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) consensus: Save previous last-logged OpId across tablet copies
Dan Burkert has posted comments on this change. Change subject: consensus: Save previous last-logged OpId across tablet copies .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) consensus: Save previous last-logged OpId across tablet copies
Dan Burkert has submitted this change and it was merged. Change subject: consensus: Save previous last-logged OpId across tablet copies .. consensus: Save previous last-logged OpId across tablet copies When a replica falls behind the leader's log, and then fails a tablet copy, it should still be able to vote in leader elections while tombstoned. With this patch, that becomes possible. This helps with availability. Without this patch, asking a post-tablet copy failure replica to vote would result in the following RPC response: IllegalState: must be running to vote when last-logged opid is not known Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Reviewed-on: http://gerrit.cloudera.org:8080/7888 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: David Ribeiro Alves Reviewed-on: http://gerrit.cloudera.org:8080/7912 Reviewed-by: Dan Burkert --- M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/tablet/tablet_metadata.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 5 files changed, 81 insertions(+), 13 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie886764adbdc6d84406df44d6cdb693d038bcd57 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) tablet copy: Allow voting from failed initial tablet copies
Dan Burkert has submitted this change and it was merged. Change subject: tablet copy: Allow voting from failed initial tablet copies .. tablet copy: Allow voting from failed initial tablet copies This patch changes the behavior of tablets being copied for the first time, i.e. to a new tablet server that has never hosted that replica. In that case, if the tablet copy fails, with this patch that replica will still be allowed to vote while tombstoned. This improves availability. Without this patch, if a tablet copy to a new replica fails, that replica will not be able to vote while tombstoned because its last-logged OpId field in its superblock will be unset. This is achieved within tablet copy by setting the initial tombstoned_last_logged_opid to (1,0). This patch is a bit of a hack, since it takes advantage of the fact that it is impossible to have a "real" OpId of 1.0 in the current implementation (1.1 is the operational minimum) so having a last-logged OpId of 1.0 represents having never logged any actual ops. Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Reviewed-on: http://gerrit.cloudera.org:8080/7904 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/7913 Reviewed-by: Dan Burkert --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_client.cc 2 files changed, 85 insertions(+), 0 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibaad2f56797db731a0f701bbd37e3e450bc17b51 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) KUDU-2118. Fully shut down TabletReplica on delete
Dan Burkert has submitted this change and it was merged. Change subject: KUDU-2118. Fully shut down TabletReplica on delete .. KUDU-2118. Fully shut down TabletReplica on delete After the various lifecycle changes implemented in 5bca7d8ba185d62952fb3e3163cbe88d20453da0 we were left with a case where we fail to fully shut down TABLET_DATA_DELETED replicas before dereferencing them. Because LeaderElection via ElectionDecisionCallback will hold a reference to RaftConsensus while a vote is taking place, the leader election callback running on the reactor thread may destroy the RaftConsensus object. If that occurs without RaftConsensus already being in a fully-shutdown state, the RaftConsensus destructor runs Shutdown() before destoying the object, which takes a lock and triggers an assertion via AssertWaitAllowed(). This crashes the server. This patch fixes the issue by calling Shutdown() on TABLET_DATA_DELETED replicas before dereferencing them in TSTabletManager. A concurrency test is included that spawns threads to start elections while the tablets are being deleted. This test pretty reliably fails without the included fix, and is solidly stable with the fix. Change-Id: I4a0ea46f220a10d4de680fc73d45d14426e0395e Reviewed-on: http://gerrit.cloudera.org:8080/7883 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-on: http://gerrit.cloudera.org:8080/7911 Reviewed-by: Dan Burkert --- M src/kudu/integration-tests/delete_tablet-itest.cc M src/kudu/tserver/ts_tablet_manager.cc 2 files changed, 59 insertions(+), 5 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4a0ea46f220a10d4de680fc73d45d14426e0395e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has uploaded a new patch set (#2). Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 31 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/2 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Andrew Wong has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/7914/1/docs/release_notes.adoc File docs/release_notes.adoc: PS1, Line 72: a remove PS1, Line 97: was nit: is PS1, Line 113: Tablet Server nit: it seems, at least in this doc, that we're standardizing to lower-cased component names? PS1, Line 119: Tablet Server same here? PS1, Line 139: * Kudu 1.0 clients may connect to servers running Kudu 1.5 with the exception of the : below-mentioned restrictions regarding secure clusters. nit: maybe move one down so "below-mentioned" refers to the blob immediately below it? -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](branch-1.5.x) java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7915 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-HasComments: No
[kudu-CR] java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has submitted this change and it was merged. Change subject: java-client: improve error messages when failing to connect to secure cluster .. java-client: improve error messages when failing to connect to secure cluster Cleans up the error handling in Connection.java so that is passes through non-recoverable exceptions, and improves the error which results from attempting to connect to a secure cluster with an unauthenticated client. Example of the new error: org.apache.kudu.client.NonRecoverableException: Couldn't find a valid master in (nightly512-1.gce.cloudera.com:7051). Exceptions received: org.apache.kudu.client.NonRecoverableException: Server requires Kerberos, but this client is not authenticated (kinit) Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Reviewed-on: http://gerrit.cloudera.org:8080/7824 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java 4 files changed, 109 insertions(+), 77 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Alexey Serbin has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 64: beteween between PS2, Line 72: values nit: maybe replace with one of parameters/properties/settings? -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](branch-1.5.x) java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has uploaded a new change for review. http://gerrit.cloudera.org:8080/7915 Change subject: java-client: improve error messages when failing to connect to secure cluster .. java-client: improve error messages when failing to connect to secure cluster Cleans up the error handling in Connection.java so that is passes through non-recoverable exceptions, and improves the error which results from attempting to connect to a secure cluster with an unauthenticated client. Example of the new error: org.apache.kudu.client.NonRecoverableException: Couldn't find a valid master in (nightly512-1.gce.cloudera.com:7051). Exceptions received: org.apache.kudu.client.NonRecoverableException: Server requires Kerberos, but this client is not authenticated (kinit) Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Reviewed-on: http://gerrit.cloudera.org:8080/7824 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins (cherry picked from commit 986e8de63d8687421c476de07c4e889129062637) --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java 4 files changed, 109 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/7915/1 -- To view, visit http://gerrit.cloudera.org:8080/7915 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hao Hao has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 76: tablet move tool 'tablet move' tool PS2, Line 79: local replica data size 'local_replica_data_size' PS2, Line 84: functionality functionalities PS2, Line 88: functionality functionalities -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Alexey Serbin has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 124: * The Java Kudu client now automatically requests new authentication tokens : after expiration. So, long-lived Java clients are now supported. See : link:https://issues.apache.org/jira/browse/KUDU-2013[KUDU-2013] for more : details. Should this be moved under the 'Fixed Issues' instead? -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/7914/1/docs/release_notes.adoc File docs/release_notes.adoc: PS1, Line 72: a > remove Done PS1, Line 97: was > nit: is Done PS1, Line 113: Tablet Server > nit: it seems, at least in this doc, that we're standardizing to lower-case Done PS1, Line 119: Tablet Server > same here? Done PS1, Line 139: * Kudu 1.0 clients may connect to servers running Kudu 1.5 with the exception of the : below-mentioned restrictions regarding secure clusters. > nit: maybe move one down so "below-mentioned" refers to the blob immediatel Done http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 64: beteween > between Done PS2, Line 72: values > nit: maybe replace with one of parameters/properties/settings? Done PS2, Line 84: functionality > functionalities I think both are correct, functionality can cover many items in American english. PS2, Line 124: * The Java Kudu client now automatically requests new authentication tokens : after expiration. So, long-lived Java clients are now supported. See : link:https://issues.apache.org/jira/browse/KUDU-2013[KUDU-2013] for more : details. > Should this be moved under the 'Fixed Issues' instead? Yah good catch. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7914 to look at the new patch set (#3). Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 43 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/3 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 1: This keeps running into flakes, manually verifying. -- To view, visit http://gerrit.cloudera.org:8080/7915 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7915 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) java-client: improve error messages when failing to connect to secure cluster
Dan Burkert has submitted this change and it was merged. Change subject: java-client: improve error messages when failing to connect to secure cluster .. java-client: improve error messages when failing to connect to secure cluster Cleans up the error handling in Connection.java so that is passes through non-recoverable exceptions, and improves the error which results from attempting to connect to a secure cluster with an unauthenticated client. Example of the new error: org.apache.kudu.client.NonRecoverableException: Couldn't find a valid master in (nightly512-1.gce.cloudera.com:7051). Exceptions received: org.apache.kudu.client.NonRecoverableException: Server requires Kerberos, but this client is not authenticated (kinit) Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Reviewed-on: http://gerrit.cloudera.org:8080/7824 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins (cherry picked from commit 986e8de63d8687421c476de07c4e889129062637) Reviewed-on: http://gerrit.cloudera.org:8080/7915 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java 4 files changed, 109 insertions(+), 77 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/7915 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Alexey Serbin has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (2 comments) Just a couple of other nits; sorry that I missed those in the previous pass. Feel free to ignore, though. http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 92: Kudu 1.5's log block manager now performs disk synchronization in batches. consider: 'Starting Kudu 1.5, the log block manager performs disk synchronization in batches.' or 'The log block manager now performs disk synchronization in batches.' PS2, Line 97: feature nit: does it make sense to mention that the feature is still in 'experimental' phase? Like 'A new experimental feature referred as ...' -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7914/2/docs/release_notes.adoc File docs/release_notes.adoc: PS2, Line 92: Kudu 1.5's log block manager now performs disk synchronization in batches. > consider: Done PS2, Line 97: feature > nit: does it make sense to mention that the feature is still in 'experiment I don't think so in this case, since the feature itself is not really experimental (it's enabled by default), only the flag is. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Alexey Serbin has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/7914/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 72: values nit: it's still 'values', but if you think that's better wording, I'm fine with that since I'm not a native English speaker :) Line 132: Probably, that's out of the intended scope of this review item, but are you going to add mention on fixed issues like KUDU-2032, KUDU-1942, and KUDU-2085? -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7914/3/docs/release_notes.adoc File docs/release_notes.adoc: PS3, Line 72: values > nit: it's still 'values', but if you think that's better wording, I'm fine Yes, I think values is correct here since there are multiple configuration options exposed. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Andrew Wong has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
