Todd Lipcon has posted comments on this change. Change subject: WIP: tie log retention to consensus watermarks ......................................................................
Patch Set 1: (3 comments) just responding to comments. will work on the TODOs in the patch and add tests. http://gerrit.cloudera.org:8080/#/c/4177/1//COMMIT_MSG Commit Message: Line 60: WIP because this needs some automated tests for the above behavior, and > would also be good to get an idea on what is the impact on startup. our boo will add a comment to the commit message about this. http://gerrit.cloudera.org:8080/#/c/4177/1/src/kudu/consensus/log.cc File src/kudu/consensus/log.cc: Line 650: rem_segs <= FLAGS_log_max_segments_to_retain) { > idea (possibly unrelated to this patch), instead of a max segments to retai hesitant to do it percentage-wise, because it's not always obvious how that plays with shared drive usage. ie is it "20% of the total drive?" or "20% of the space that you want Kudu to be using"? I do agree that size-wise is better than a segment count, and it should probably be global across all tablets, not a per-tablet setting. If you don't mind, I'll try to tackle this as part of KUDU-1567. I did mark the new 'max_segments_to_retain' as experimental so that we can break it. http://gerrit.cloudera.org:8080/#/c/4177/1/src/kudu/consensus/raft_consensus.h File src/kudu/consensus/raft_consensus.h: Line 173: log::RetentionIndexes GetLogRetention() override; > rename to GetRetentionIndexes? Done -- To view, visit http://gerrit.cloudera.org:8080/4177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfc071270510f3dc3c65f88d615e93c6ffb26b12 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
