Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9452 )
Change subject: KUDU-2293 fix cleanup after failed copies ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/9452/9/src/kudu/tserver/tablet_copy_client.h File src/kudu/tserver/tablet_copy_client.h: http://gerrit.cloudera.org:8080/#/c/9452/9/src/kudu/tserver/tablet_copy_client.h@148 PS9, Line 148: kStarting > Since we are still conditioning on meta_, I don't see the purpose of this n meta_ gets set in SetTabletToReplace(), which happens before Start(). While in this state, meta_ may have been updated, and its state will be TABLET_DATA_COPYING, and so if we Abort() while in this state, it needs to be cleaned up. The difference between this and kStarted is that kStarted indicates that Start() has completed completely. E.g. it'd be a programming error to start copying data while in kStarting, but it's correct to do so while in kStarted. http://gerrit.cloudera.org:8080/#/c/9452/9/src/kudu/tserver/tablet_copy_client.h@157 PS9, Line 157: kFinishing > What is the purpose of this state? Do we need it? We could do away with this and replace setting kFinishing with kFinished in Finish(). If we do that, then kFinished won't actually mean "finished" -- it'll mean "we've finished copying everything over and may still need to set the data to TABLET_DATA_READY". Maybe that's ok. WDYT? http://gerrit.cloudera.org:8080/#/c/9452/9/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: http://gerrit.cloudera.org:8080/#/c/9452/9/src/kudu/tserver/tablet_copy_client.cc@440 PS9, Line 440: CHECK_EQ(tablet::TABLET_DATA_TOMBSTONED > what guarantees this? Success or fail, DeleteTabletData() leaves the tablet in TABLET_DATA_TOMBSTONED. Maybe we could split that into some `meta_->SetTabletState()` that would make the state change more explicit and a `DeleteTabletData()`. -- 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: 9 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Fri, 06 Apr 2018 01:34:26 +0000 Gerrit-HasComments: Yes