Adar Dembo has posted comments on this change. Change subject: WIP: Add a tool to purge UNDOs, and one to summarize data size ......................................................................
Patch Set 1: (6 comments) Here's your *it probably won't eat your data* seal of approval. http://gerrit.cloudera.org:8080/#/c/5727/1/src/kudu/tablet/rowset_metadata.cc File src/kudu/tablet/rowset_metadata.cc: Line 205: undo_delta_blocks_.insert(undo_delta_blocks_.begin(), update.new_undo_block_); As a sanity check, we should probably make sure that new_undo_block_ isn't in remove_undo_blocks_. http://gerrit.cloudera.org:8080/#/c/5727/1/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: Line 334: FsManager fs_manager(Env::Default(), FsManagerOpts()); Can use FsInit() here too? Line 340: if (!MatchPattern(tablet_id, tablet_id_pattern)) continue; Should probably fully document the pattern matching semantics in the tool's help (i.e. incorporate '?', talk about escaping, talk about limits, etc) PS1, Line 375: cout << block_type << " block " << b.ToString() << ": " : << size << " bytes "<< HumanReadableNumBytes::ToString(size) << endl; Would be nice to relegate this to "--verbose" or some such. Line 381: Status SummarizeDataSize(const RunnerContext& context) { As we discussed, it would also be nice to be able to sum up redo or undo or bloom or index for a particular tablet, or across all tablets. Line 411: cout << "Rowset total: " << HumanReadableNumBytes::ToString(rowset_sum) << endl; This one too. -- To view, visit http://gerrit.cloudera.org:8080/5727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf0237f9d01f4ec332d1df545d2c2f51f64ce012 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
