Andrew Wong has posted comments on this change.

Change subject: Allow tablet shutdown without completing txs

Patch Set 8:


Also as per our slack discussion, rather than making use of tablet::FAILED, 
which in I'm making a "final" state, I 
went with your suggestion of shutting down the MvccManager. There's still a 
separation of replica and tablet, but I think this at least makes sense w.r.t. 

Also note in on of my review comments that the MvccManager::SetShuttingDown() 
isn't really used here, but will be in the ErrorNotificationCb.
File src/kudu/tablet/mvcc.h:

Line 404:   void Cancel();
> here's an idea: how about rather than specifically cancelling each transact
File src/kudu/tablet/transactions/transaction.h:

Line 20: #include <mutex>
> warning: #includes are not sorted properly [llvm-include-order]
File src/kudu/tablet/transactions/

Line 481:       CHECK(s.IsDiskFailure());
> per conversation on Slack, I think generalizing this to something like the 
I updated HandleFailure to allow replicating/replicated failures to go through 
if the tablet is shutting down. The drive is able to get the necessary info 
(i.e. whether mvcc is shut down)  through the TransactionState.
File src/kudu/tablet/transactions/

Line 134:   if (PREDICT_FALSE(tablet->IsInFailedDir())) {
> making ApplyRowOperations return a Status would allow centralizing this log

Line 139:   tablet->ApplyRowOperations(state());
> here if Tablet indicated that it has shut down then you can early-return wi
File src/kudu/tserver/

Line 1076:     tablet->SetInFailedDir();
> again maybe generalize to just 'MarkFailed' since the handling isn't partic
Since this codepath is, in this patch, still fatal, I've reverted these 
changes. The code here *would* be:


Line 1078:   LOG(FATAL) << Substitute("Data directory $0 failed. Disk failure 
handling not implemented",
> now implemented?
Not quite, I moved other parts of failure handling (i.e. submission of 
Shutdown() call, permissiveness failed Flushes) to another patch to be "easier 
to review," and kept this fatal. Only updated here to show where the new code 
would be used.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I983620f27e7226806a2cca253db7619731914d42
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <>
Gerrit-Reviewer: Andrew Wong <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to