Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8441 )
Change subject: tablet: make various update paths atomic ...................................................................... Patch Set 5: (9 comments) http://gerrit.cloudera.org:8080/#/c/8441/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8441/4//COMMIT_MSG@9 PS4, Line 9: he tablet > nit: This seems like more of a requirement than an invariant to me. An inva Ack http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/delta_compaction.cc File src/kudu/tablet/delta_compaction.cc: http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/delta_compaction.cc@372 PS4, Line 372: > Please add a high level comment to this function regarding when we're allow Done http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/delta_compaction.cc@386 PS4, Line 386: RETURN_NOT_OK(tracker->OpenDeltaReaders(new_undo_blocks, &new_undo_stores, UNDO)); > Add: Done http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/delta_compaction.cc@388 PS4, Line 388: > I'm curious if the order in which we update the stores matters. If so, it d I don't think so given this all happens under locks. http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/delta_compaction.cc@390 PS4, Line 390: > insert: for UNDOs Done http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/diskrowset.cc@a577 PS4, Line 577: : : > why was this comment removed? Done http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/diskrowset.cc@567 PS4, Line 567: > what does out of sync mean? Done http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/diskrowset.cc@569 PS4, Line 569: > By whom? What kind of guarantee do we get? Took this out and replaced with CHECKs. http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/tablet_mm_ops.cc File src/kudu/tablet/tablet_mm_ops.cc: http://gerrit.cloudera.org:8080/#/c/8441/4/src/kudu/tablet/tablet_mm_ops.cc@264 PS4, Line 264: oped_refptr<AtomicGauge<uint32_t> > MajorDeltaCompactionOp::RunningGauge() const { : return tablet_->metrics()->delta_major_compact_rs_running; : } : : //////////////////////////////////////////////////////////// : // > This logic is reasonable up at this high-level driver code but it should be Ack -- To view, visit http://gerrit.cloudera.org:8080/8441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0241cd4206ce77ef1fb334458b091bc2092f4141 Gerrit-Change-Number: 8441 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 17 Nov 2017 17:45:53 +0000 Gerrit-HasComments: Yes
