[kudu-CR] KUDU-2293 fix cleanup after failed copies

["Mike Percy (Code Review)" <ge...@cloudera.org>]

Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9452 )

Change subject: KUDU-2293 fix cleanup after failed copies
......................................................................


Patch Set 5:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9452/5/src/kudu/tserver/tablet_copy_client.cc@302
PS5, Line 302:   cleanup_cb_ = function<void(void)>([&] {
I feel like this class would be easier to follow if we just always called Abort() in the destructor like before and keep track of whether we're kStarting or better, or something like that. What do you think? It seems like part of the problem in this class is that the values of the state_ enum aren't really sufficient for all cases.


http://gerrit.cloudera.org:8080/#/c/9452/5/src/kudu/tserver/tablet_copy_client.cc@459
PS5, Line 459:   RETURN_NOT_OK(meta_->LoadFromSuperBlock(*superblock_));
Hmm, why load now and not replace anymore? Doesn't this make it more likely to orphan blocks on crash?

Then again, we are about to flush the superblock in DeleteTabletData(), so maybe it's not required.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0459d484a3064aa2de392328e2910c9f6741be04
Gerrit-Change-Number: 9452
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 02 Apr 2018 08:55:18 +0000
Gerrit-HasComments: Yes