You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/09/07 21:20:40 UTC

[kudu-CR] WIP: tie log retention to consensus watermarks

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 <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes