Dinesh Bhat has posted comments on this change. Change subject: [tools] Tombstone the tablet with "local_replica delete" ......................................................................
Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/5191/2/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: Line 166: Status FindLastCommittedOpId(FsManager* fs, const string& tablet_id, > 0) Yeah I saw that after reading this part, so it's certainly not crazy, bu Thanks Mike for explanations, I modified the paramter as per above discussion. http://gerrit.cloudera.org:8080/#/c/5191/3/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: PS3, Line 185: : > You don't need the ": " part, that is automatically handled for you. If you Cool, tx. Line 185: s.CloneAndPrepend("Corruption in log segment: "); > This is a no-op... s is not mutated, it is cloned and returned. That would upset tidy-bot [readability-issue-with-else-after-return] or something like that. I think that was the partial reason why I had s = Status:: earlier there. I think RETURN_NOT_OK_PREPEND is a good choice, updated. Line 308: TabletDataState state; > How about default state = TabletDataState::TABLET_DATA_DELETED and skip the Done Line 314: if (s.ok() || s.IsNotFound()) { > How about: Nice, took that verbatim. PS3, Line 315: state = TabletDataState::TABLET_DATA_TOMBSTONED > This should always be set as part of if (!FLAGS_clean_unsafe), you can move Done -- To view, visit http://gerrit.cloudera.org:8080/5191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia773de475431eb85fb0dbe724d524e8dd59b1b12 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
