You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kudu.apache.org by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org> on 2016/03/02 05:32:26 UTC

[kudu-CR](branch-0.7.x) KUDU-1325: more crash avoidance during remote bootstrap and tablet deletion

Hello Todd Lipcon, Kudu Jenkins,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/2389

to review the following change.

Change subject: KUDU-1325: more crash avoidance during remote bootstrap and tablet deletion
......................................................................

KUDU-1325: more crash avoidance during remote bootstrap and tablet deletion

Just as with KUDU-1328, here is yet another instance where remote bootstrap
doesn''t properly protect itself in the event of a race with tablet
deletion. KUDU-1337 is responsible for this sequence of events happening so
often, but it's worth addressing even if it were rare.

This time around, we'll deal with it by giving LogReader shared ownership.
Personally this feels like overkill (extra cognitive load when dealing with
LogReader just to prevent this race?) but it's the easiest to implement
given the design of the server-side part of remote bootstrap. I'm still
working on reproducing these races reliably in an integration test.

What else is interesting? Turns out std::make_shared() can't be used on
classes with private constructors. I've added a workaround in
ALLOW_MAKE_SHARED which is simple but entirely non-portable.

Change-Id: I6a01c9e6886bd8bf0e286fc3980babc512056286
Reviewed-on: http://gerrit.cloudera.org:8080/2265
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/consensus/log-dump.cc
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/consensus/mt-log-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tserver/remote_bootstrap_client-test.cc
M src/kudu/tserver/remote_bootstrap_service-test.cc
M src/kudu/tserver/remote_bootstrap_session.cc
A src/kudu/util/make_shared.h
16 files changed, 174 insertions(+), 93 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/2389/1
-- 
To view, visit http://gerrit.cloudera.org:8080/2389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a01c9e6886bd8bf0e286fc3980babc512056286
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.7.x
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>