Hao Hao has posted comments on this change. Change subject: KUDU-1943: Add BlockTransaction to Block Manager ......................................................................
Patch Set 16: > (5 comments) > > Hao, Dan, and I had a long discussion about this patch, and I > wanted to reproduce our conclusions here for everyone else: > > 1. The cfile and block performance-related knobs are useful for > testing, but are not as important as having a good API and a robust > implementation. IIUC, the resplitting of FlushDataAsync and > Finalize was due to retaining all the existing flags; let's not do > this. Let's stick to the earlier approach (where Finalize implies a > flush), and adjust the flags if need be. > > 2. What flags are worth preserving? We identified the need for just > one, which dictates when a block's data should be flushed. It has > three values: "finalize" (default value), "close" (defers the flush > to when the entire transaction is committed), and "none" (doesn't > flush at all, "flushing" will happen when the transaction commits > and files are fsynced). > > 3. When should AppendMetadata be called? One option is to do it at > Finalize time. Another is to defer it to Close. The problem with > doing it during Finalize is that it exacerbates the "1. AppendData, > 2. AppendMetadata, 3. SyncData, 4. SyncMetadata" race: if we crash > between #2 and #3, we may end up with just the metadata on disk, > and if we AppendMetadata during Finalize, we have many more blocks' > worth of metadata that can land on disk without corresponding data. > > 4. But AppendMetadata is called during Close, we may end up with > out-of-order metadata entries on disk, since transactions could > commit out of order. Do we care? We don't think so; the LBM > implementation should already be robust to this. An alternative is > to use an in-memory buffer to retain metadata order, but this > didn't seem worth the complexity. > > 5. What to do about zero length blocks? Should their metadata be > written, or skipped? The answer is: it must be written, or attempts > to Close such blocks should fail. If for whatever reason we skipped > the metadata append, we could end up with block IDs whose blocks > can't be found on startup. Thanks a lot Adar for the summary! -- To view, visit http://gerrit.cloudera.org:8080/7207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I01f3ed830cc1f86d7bb8a66b2f780914a88884bd Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
