[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for ConsensusMetadataManager::LoadOrCreate().
* A unit test for ConsensusMetadataCreateMode::NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass, and an additional test was
added for unflushed cmeta instances.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Reviewed-on: http://gerrit.cloudera.org:8080/7988
Reviewed-by: Andrew Wong 
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 370 insertions(+), 67 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, but someone else must approve
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 9: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 9: Code-Review+1

(2 comments)

lgtm; Alexey may want to take another look considering cmeta_manager-test has 
changed a little.

http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS8, Line 109: 
RETURN_NOT_OK_PREPEND(ConsensusMetadata::DeleteOnDiskData(fs_manager_, 
tablet_id),
 : Substitute("Unable to delete consensus 
metadata for tablet $0", tablet_id));
> That's true. I've just updated the header file comment to note that NotFoun
Ah right, gotcha


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS7, Line 1088: guarantees that the replica has never voted in a leader 
election.
> I think I agree with what you said. But when it comes to consensus, you hav
Hah, you're right. I guess "normal" for the purposes of my comment would be 
where no one is intentionally deleting and recreating cmeta files for tablets 
that have voted already.

That sort of thing generally doesn't happen, and if it never does, the 
assumption should always be true: the nonexistence of a cmeta file implies that 
the replica has not voted.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS7, Line 71: ConsensusMetadataCreateMode create_mode =
:   
ConsensusMetadataCreateMode::FLUSH_ON_CREATE,
> Hrm I'm specifically asking about LoadOrCreate, not Create--the first call 
aha. yes.


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS7, Line 1088: guarantees that the replica has never voted in a leader 
election.
> Makes sense; so for the "normal" use case, this assumption is not so much n
I think I agree with what you said. But when it comes to consensus, you have to 
be a real language lawyer, and I'm not sure what you mean by the "normal" use 
case.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS8, Line 109: 
RETURN_NOT_OK_PREPEND(ConsensusMetadata::DeleteOnDiskData(fs_manager_, 
tablet_id),
 : Substitute("Unable to delete consensus 
metadata for tablet $0", tablet_id));
> Doesn't this fail if the cmeta is in-memory only? Is that something we shou
That's true. I've just updated the header file comment to note that NotFound 
will be returned if the data isn't found on disk. It's not really an issue in 
practice, because the only caller (ts_tablet_manager.cc) considers 
s.IsNotFound() to be an acceptable success code from this method, per the 
header doc.


http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS8, Line 62: the cache
> nit: hrmm, since "the cache" might be confused with something Kudu-specific
Done


PS8, Line 67:  otherwise create i
> nit: Maybe also note the insignificance of the parameters if a Load is expe
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#9).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for ConsensusMetadataManager::LoadOrCreate().
* A unit test for ConsensusMetadataCreateMode::NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass, and an additional test was
added for unflushed cmeta instances.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 370 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/9
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS8, Line 109: 
RETURN_NOT_OK_PREPEND(ConsensusMetadata::DeleteOnDiskData(fs_manager_, 
tablet_id),
 : Substitute("Unable to delete consensus 
metadata for tablet $0", tablet_id));
Doesn't this fail if the cmeta is in-memory only? Is that something we should 
be worried about?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS7, Line 71: ConsensusMetadataCreateMode create_mode =
:   
ConsensusMetadataCreateMode::FLUSH_ON_CREATE,
> That's something that is caught by the cache check, so the first call will 
Hrm I'm specifically asking about LoadOrCreate, not Create--the first call 
Creates and doesn't flush, the second loads from cache. And in the second call, 
the create_mode doesn't matter.

Not an actionable comment, just checking my understanding :)


http://gerrit.cloudera.org:8080/#/c/7988/8/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS8, Line 62: the cache
nit: hrmm, since "the cache" might be confused with something Kudu-specific 
(e.g. block cache, or other caches if we ever implement them), consider 
"in-memory"? I don't feel too strongly about this, but thought I'd point it out.


PS8, Line 67:  otherwise create i
nit: Maybe also note the insignificance of the parameters if a Load is expected?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS7, Line 1088: guarantees that the replica has never voted in a leader 
election.
> It's critical. If it's not true, we lose our Raft safety guarantees and spl
Makes sense; so for the "normal" use case, this assumption is not so much 
necessary as it is just how things are, and if the assumption doesn't hold, we 
might end up in some bizarro Raft scenario.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 7:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/7988/7//COMMIT_MSG
Commit Message:

Line 40: 
> nit: do you want to mention new tests for CreateOrLoad()?
Done


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta-test.cc
File src/kudu/consensus/consensus_meta-test.cc:

PS7, Line 121:  N
> nit: should be lowercased
So I looked it up, and apparently you are right.


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta.h
File src/kudu/consensus/consensus_meta.h:

PS7, Line 175: is flushed to disk before returning
> This is no longer always true.
Thanks for the catch. I'll update this.


PS7, Line 185: object from disk.
> IIUC, Create(NO_FLUSH_ON_CREATE) followed by Load() is an error, right? Is 
It's not a problem in practice because end-users of this API always go through 
the ConsensusMetadataManager to acquire a handle to one of these cmeta objects.


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager-test.cc
File src/kudu/consensus/consensus_meta_manager-test.cc:

PS7, Line 103: Load
> Would using LoadOrCreate here as well be appropriate?
Good call. Done.


PS7, Line 138:   {
> nit: this additional scope might be removed, I think.
Done


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS7, Line 62: Returns an error if it cannot be found.
> Kind of follows from my other question: IIUC, Create(NO_FLUSH_ON_CREATE) fo
Good catch; updated the comment.


PS7, Line 71: ConsensusMetadataCreateMode create_mode =
:   
ConsensusMetadataCreateMode::FLUSH_ON_CREATE,
> Just making sure I understand this, back-to-back calls to LoadOrCreate(NO_F
That's something that is caught by the cache check, so the first call will 
succeed but the second call will fail. It's tested in 
ConsensusMetadataManagerTest.TestCreateMultipleUnFlushedCMetas


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS7, Line 1088: guarantees that the replica has never voted in a leader 
election.
> I think I understand why this is true (voting --> flushing to disk). Does a
It's critical. If it's not true, we lose our Raft safety guarantees and 
split-brain becomes possible because the same replica can vote differently 
multiple times in the same term. In practice, if it happened that split brain 
scenario would probably be rare, but Murphy's Law and all that.

Todd and I discussed this on Slack and in person and we decided that there's 
only so much one can do... so we will essentially consider someone deleting 
critical files out from under us a type of byzantine failure, which means it's 
undefined behavior as far as Raft is concerned. It's very hard to do something 
reasonable to protect against stuff like that.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#8).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for ConsensusMetadataManager::LoadOrCreate().
* A unit test for ConsensusMetadataCreateMode::NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass, and an additional test was
added for unflushed cmeta instances.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 366 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta-test.cc
File src/kudu/consensus/consensus_meta-test.cc:

PS7, Line 121:  N
nit: should be lowercased


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta.h
File src/kudu/consensus/consensus_meta.h:

PS7, Line 175: is flushed to disk before returning
This is no longer always true.


PS7, Line 185: object from disk.
IIUC, Create(NO_FLUSH_ON_CREATE) followed by Load() is an error, right? Is this 
by design / is it at all important?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager-test.cc
File src/kudu/consensus/consensus_meta_manager-test.cc:

PS7, Line 103: Load
Would using LoadOrCreate here as well be appropriate?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS7, Line 62: Returns an error if it cannot be found.
Kind of follows from my other question: IIUC, Create(NO_FLUSH_ON_CREATE) 
followed by Load() is NOT an error here, right? If so, is this difference in 
APIs by design? It's a subtle difference that might be worth documenting.


PS7, Line 71: ConsensusMetadataCreateMode create_mode =
:   
ConsensusMetadataCreateMode::FLUSH_ON_CREATE,
Just making sure I understand this, back-to-back calls to 
LoadOrCreate(NO_FLUSH_ON_CREATE) is not an error, right? It will just result in 
nothing on disk?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS7, Line 1088: guarantees that the replica has never voted in a leader 
election.
I think I understand why this is true (voting --> flushing to disk). Does 
anything hinge on this assumption being true?


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 7: Code-Review+2

(3 comments)

> (6 comments)
 > 
 > > (7 comments)
 > >
 > > I have the following question: why do we need to introduce that
 > > additional state into ConsensusMetadata object? Why not to leave
 > > the OVERWRITE/NO_OVERWRITE parameter for its Flush() method?
 > 
 > After discussing this with you offline, I agree that we can
 > simplify the implementation and avoid this additional state. I'll
 > back out this portion of the patch.

Thank you for addressing my concerns!

As for now, LGTM, just a couple of nits.  Feel free to ignore, though.

http://gerrit.cloudera.org:8080/#/c/7988/7//COMMIT_MSG
Commit Message:

Line 40: 
nit: do you want to mention new tests for CreateOrLoad()?


http://gerrit.cloudera.org:8080/#/c/7988/7/src/kudu/consensus/consensus_meta_manager-test.cc
File src/kudu/consensus/consensus_meta_manager-test.cc:

PS7, Line 138:   {
nit: this additional scope might be removed, I think.


http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS6, Line 127: using consensus::ConsensusMetadataCreateMode;
> This one is needed :)
oh, now I see -- that's for LoadOrCreate() call; it seems I didn't scroll down 
enough to see that :)


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#7).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass, and an additional test was
addedadded for unflushed cmeta instances.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 352 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/7
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 6:

(6 comments)

> (7 comments)
 > 
 > I have the following question: why do we need to introduce that
 > additional state into ConsensusMetadata object? Why not to leave
 > the OVERWRITE/NO_OVERWRITE parameter for its Flush() method?

After discussing this with you offline, I agree that we can simplify the 
implementation and avoid this additional state. I'll back out this portion of 
the patch.

http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/consensus/consensus_meta.cc
File src/kudu/consensus/consensus_meta.cc:

PS6, Line 334: /*overwrite_first_flush=*/ false
> Why not just to pass the NO_OVERWRITE flag to the Flush() method as it was 
The semantics I want to maintain are that we don't overwrite on the first 
Flush() call after create, but we overwrite at any other time.


http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

Line 96: return Create(tablet_id, config, initial_term, create_mode);
> Does it make sense to pass the cmeta_out here as well?
Fixed, added a unit test too.


http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/tablet/tablet_replica-test.cc
File src/kudu/tablet/tablet_replica-test.cc:

PS6, Line 86: using consensus::ConsensusMetadataCreateMode;
> nit: is it needed?
Done


http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/tserver/tablet_copy_source_session-test.cc
File src/kudu/tserver/tablet_copy_source_session-test.cc:

PS6, Line 91: using consensus::ConsensusMetadataCreateMode;
> nit: is it needed?
Done


http://gerrit.cloudera.org:8080/#/c/7988/6/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS6, Line 127: using consensus::ConsensusMetadataCreateMode;
> nit: is it needed?
This one is needed :)


PS6, Line 1093: scoped_refptr cmeta;
> nit: it seems the 'cmeta' is not used in this scope; drop it, maybe?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#6).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Simplify the interface controlling whether a Flush() is allowed to
  overwrite (clobber) an existing file by encapsulating that logic in
  the implementation, instead of the public interface to
  ConsensusMetadata. When a cmeta is instantiated via
  ConsensusMetadata::Create(), the next Flush() is not allowed to
  overwrite an existing file. In every other case, Flush() is allowed to
  overwrite an existing file.
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 345 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#5).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Simplify the interface controlling whether a Flush() is allowed to
  overwrite (clobber) an existing file by encapsulating that logic in
  the implementation, instead of the public interface to
  ConsensusMetadata. When a cmeta is instantiated via
  ConsensusMetadata::Create(), the next Flush() is not allowed to
  overwrite an existing file. In every other case, Flush() is allowed to
  overwrite an existing file.
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 345 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#4).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Simplify the interface controlling whether a Flush() is allowed to
  overwrite (clobber) an existing file by encapsulating that logic in
  the implementation, instead of the public interface to
  ConsensusMetadata. When a cmeta is instantiated via
  ConsensusMetadata::Create(), the next Flush() is not allowed to
  overwrite an existing file. In every other case, Flush() is allowed to
  overwrite an existing file.
* Made some additional method arguments optional, for convenience.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
15 files changed, 345 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 3:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta.cc
File src/kudu/consensus/consensus_meta.cc:

PS3, Line 339: // Create() should not clobber.
> I think the placement of this comment might be a bit confusing. Should this
removed this comment


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager-stress-test.cc
File src/kudu/consensus/consensus_meta_manager-stress-test.cc:

PS3, Line 18: #include 
> nit: prefer using 
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS3, Line 97:   if (s.ok()) 
> Did you consider narrowing the permissible errors here? e.g. if we hit an I
Thanks for the catch. That's what I meant to do. Fixed.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS3, Line 59: ConsensusMetadataCreateMode create_mode
> What if this parameter was set to ConsensusMetadataCreateMode::FLUSH_ON_CRE
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/raft_consensus_quorum-test.cc
File src/kudu/consensus/raft_consensus_quorum-test.cc:

PS3, Line 191: scoped_refptr cmeta;
> nit: drop this and leave the last parameter of the Create() call below by d
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/integration-tests/tombstoned_voting-itest.cc
File src/kudu/integration-tests/tombstoned_voting-itest.cc:

PS3, Line 18: #include 
> nit: consider replacing with  and adding that into the list of the
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

PS3, Line 260: scoped_refptr cmeta;
> nit: consider dropping this since it's not used in the scope and leaving th
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:

PS3, Line 146: scoped_refptr cmeta;
> Nit: it seems this is not used, consider dropping it and leaving the last p
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_replica-test.cc
File src/kudu/tablet/tablet_replica-test.cc:

PS3, Line 141: scoped_refptr cmeta;
> Nit: consider dropping this since it's not used, leaving the last parameter
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

PS3, Line 601: scoped_refptr cmeta;
> Nit: cmeta is not used in this scope, right?  Consider dropping it.
Done


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_source_session-test.cc
File src/kudu/tserver/tablet_copy_source_session-test.cc:

PS3, Line 157: scoped_refptr cmeta;
> nit: consider dropping cmeta -- it's not used in this scope elsewhere but p
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta.cc
File src/kudu/consensus/consensus_meta.cc:

PS3, Line 339: // Create() should not clobber.
I think the placement of this comment might be a bit confusing. Should this 
comment be above L333, since that's where overwrite_first_flush is specified?


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.cc
File src/kudu/consensus/consensus_meta_manager.cc:

PS3, Line 97:   if (s.ok()) 
Did you consider narrowing the permissible errors here? e.g. if we hit an IO 
error when we Load(), but successfully create the in-memory cmeta, we can get 
by without addressing the error at all until the next flush. I suppose this 
probably won't affect correctness, but it might be worth thinking about what 
errors we can expect here, if you haven't already.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 3:

(9 comments)

Just some nits for a while -- will take a deeper 'semantic' look shortly.

http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager-stress-test.cc
File src/kudu/consensus/consensus_meta_manager-stress-test.cc:

PS3, Line 18: #include 
nit: prefer using 

I'm planning to update IWYU to give more straightforward suggestions.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/consensus_meta_manager.h
File src/kudu/consensus/consensus_meta_manager.h:

PS3, Line 59: ConsensusMetadataCreateMode create_mode
What if this parameter was set to ConsensusMetadataCreateMode::FLUSH_ON_CREATE 
by default?  That would allow for removal of some LOC in 
consensus_meta_manager-test.cc and other places where FLUS_ON_CREATE is the 
desired behavior and the cmeta_out is not needed.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/consensus/raft_consensus_quorum-test.cc
File src/kudu/consensus/raft_consensus_quorum-test.cc:

PS3, Line 191: scoped_refptr cmeta;
nit: drop this and leave the last parameter of the Create() call below by 
default.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/integration-tests/tombstoned_voting-itest.cc
File src/kudu/integration-tests/tombstoned_voting-itest.cc:

PS3, Line 18: #include 
nit: consider replacing with  and adding that into the list of the STD 
headers just below.

I'm sorry for the inconvenience right now, but I'm planning to update 
IWYU-related stuff accordingly to make its suggestions more straightforward.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

PS3, Line 260: scoped_refptr cmeta;
nit: consider dropping this since it's not used in the scope and leaving the 
last parameter of the Create() call below by default.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:

PS3, Line 146: scoped_refptr cmeta;
Nit: it seems this is not used, consider dropping it and leaving the last 
parameter for the Create() call below by default.  Also, as mentioned 
elsewhere, consider having the second-from-the-tail parameter for Create() by 
default as well.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tablet/tablet_replica-test.cc
File src/kudu/tablet/tablet_replica-test.cc:

PS3, Line 141: scoped_refptr cmeta;
Nit: consider dropping this since it's not used, leaving the last parameter for 
the Create() call below at its default value.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

PS3, Line 601: scoped_refptr cmeta;
Nit: cmeta is not used in this scope, right?  Consider dropping it.


http://gerrit.cloudera.org:8080/#/c/7988/3/src/kudu/tserver/tablet_copy_source_session-test.cc
File src/kudu/tserver/tablet_copy_source_session-test.cc:

PS3, Line 157: scoped_refptr cmeta;
nit: consider dropping cmeta -- it's not used in this scope elsewhere but 
passing as the last parameter to the Create() call below, where it can be left 
by default.


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7988/2//COMMIT_MSG
Commit Message:

PS2, Line 7: replicas
> Nit: replica
If the cmeta doesn't exist


PS2, Line 11: Not having a cmeta
: file results in those tombstoned replicas being unable to vote in 
Raft
: leader elections
> Given the conditions you spelled out in the previous sentence, it seems lik
Due to previous versions of Kudu generating many of these kinds of tombstoned 
replicas under load, it's not that rare. Additionally, we don't want them to 
show up as FAILED in the UI, so not making them as FAILED was the primary 
motivation, and being able to vote is really something of a side benefit.


PS2, Line 27: ConsensConsensusMetadataManager
> ConsensusMetadataManager
erg, my Vim auto-wrapping keeps doing this in Git messages. I need to figure 
out why, just haven't got around to it.


PS2, Line 33: thethe
> the
same, thanks


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#3).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Simplify the interface controlling whether a Flush() is allowed to
  overwrite (clobber) an existing file by encapsulating that logic in
  the implementation, instead of the public interface to
  ConsensusMetadata. When a cmeta is instantiated via
  ConsensusMetadata::Create(), the next Flush() is not allowed to
  overwrite an existing file. In every other case, Flush() is allowed to
  overwrite an existing file.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-stress-test.cc
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 357 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..


Patch Set 2:

(4 comments)

Was just passing through because I was interested.

http://gerrit.cloudera.org:8080/#/c/7988/2//COMMIT_MSG
Commit Message:

PS2, Line 7: replicas
Nit: replica

("doesn't" implies singular)


PS2, Line 11: Not having a cmeta
: file results in those tombstoned replicas being unable to vote in 
Raft
: leader elections
Given the conditions you spelled out in the previous sentence, it seems like 
not having a cmeta would be quite rare. Why is it important for these replicas 
to be able to vote?


PS2, Line 27: ConsensConsensusMetadataManager
ConsensusMetadataManager


PS2, Line 33: thethe
the


-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

[Mike Percy (Code Review)]

Hello Todd Lipcon, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#2).

Change subject: KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't 
exist at startup
..

KUDU-2123. Auto-vivify cmeta on tombstoned replicas if doesn't exist at startup

It is possible for tombstoned replicas to legitimately not have a cmeta
file as a result of crashing during a first tablet copy, or failing a
tablet copy operation in an older version of Kudu. Not having a cmeta
file results in those tombstoned replicas being unable to vote in Raft
leader elections. We remedy this by creating a cmeta object (with an
empty config) at startup time. The empty config is safe for a tombstoned
replica, because the config doesn't affect a replica's ability to vote
in a leader election. Additionally, if the tombstoned replica were ever
to be overwritten by a tablet copy operation, that would also result in
overwriting the config stored in the local cmeta with a valid Raft
config. Finally, all of this assumes that the nonexistence of a cmeta
file guarantees that the replica has never voted in a leader election.

As an optimization, the cmeta is created with the NO_FLUSH_ON_CREATE
flag, meaning that it will only be flushed to disk if the replica ever
votes.

The following changes had to be made to ConsensusMetadata and the
ConsensConsensusMetadataManager to support the above functionality:

* Enable deferred flush on Create() by defining a flag called
  NO_FLUSH_ON_CREATE
* Simplify the interface controlling whether a Flush() is allowed to
  overwrite (clobber) an existing file by encapsulating that logic in
  thethe implementation, instead of the public interface to
  ConsensusMetadata. When a cmeta is instantiated via
  ConsensusMetadata::Create(), the next Flush() is not allowed to
  overwrite an existing file. In every other case, Flush() is allowed to
  overwrite an existing file.

The following tests have been added:

* A unit test for NO_FLUSH_ON_CREATE.
* A test that crashes the target of a tablet copy after writing the
  superblock and before writing the cmeta file. The tablet server is
  restarted and the replica is expected to be able to vote while
  tombstoned.

Previously-written tests that verify ConsensusMetadata::Create() will
not clobber an existing file still pass.

Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
---
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/consensus_meta_manager-stress-test.cc
M src/kudu/consensus/consensus_meta_manager-test.cc
M src/kudu/consensus/consensus_meta_manager.cc
M src/kudu/consensus/consensus_meta_manager.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
16 files changed, 357 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/7988/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7988
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8ff6255b1fcbb12417b82853bcde9b239291492b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon