David Ribeiro Alves has posted comments on this change. Change subject: Tie log retention to consensus watermarks ......................................................................
Patch Set 5: (6 comments) lgtm mostly nits/verbage http://gerrit.cloudera.org:8080/#/c/4177/5//COMMIT_MSG Commit Message: Line 11: durability (the committed index) as well as the watermark necessary to not sure about this link between the commit index and durability, at least from a tablet perspective we might need more than the commit index for durability right? Line 18: - we always maintain any logs necessary for durability could you make this clearer? what do we do here now? calc the min between any MRS/DMS anchors (the "real" durability watermarks) and the commit index? http://gerrit.cloudera.org:8080/#/c/4177/5/src/kudu/consensus/consensus.h File src/kudu/consensus/consensus.h: Line 266: // retain. nit no need to wrap http://gerrit.cloudera.org:8080/#/c/4177/5/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: Line 1879: return log::RetentionIndexes(queue_->GetCommittedIndex(), // for durability add a note that it's ok not to get these atomically http://gerrit.cloudera.org:8080/#/c/4177/5/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h File src/kudu/integration-tests/external_mini_cluster_fs_inspector.h: Line 70: // on TS 'index'. nit no need to wrap http://gerrit.cloudera.org:8080/#/c/4177/5/src/kudu/tablet/tablet_peer.h File src/kudu/tablet/tablet_peer.h: Line 193: // up peers. nit: wrapping -- 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: 5 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
