Neeraj Singh <nksingh85@xxxxxxxxx> writes: >> IOW, I am not sure end_if_active() should exist in the first place. >> Shouldn't end_transaction() do that instead? >> > > Today there's an "assert(bulk_checkin_plugged)" in > end_odb_transaction. In principle we could just drop the assert and > allow a transaction to be ended multiple times. But maybe in the long > run for composability we'd like to have nested callers to begin/end > transaction (e.g. we could have a nested transaction around writing > the cache tree to the ODB to minimize fsyncs there). I am not convinced that "transaction" is a good mental model for this mechanism to begin with, in the sense that the sense that it is not a bug or failure of the implementation if two or more operations in the same <begin,end> bracket did not happen (or not happen) atomically, or if 'begin' and 'end' were not properly nested. With the design getting more complex with things like tentative object store that needs to be explicitly migrated after the outermost level of end-transaction, we may end up _requiring_ that sufficient number of 'end' must come once we issued 'begin', which I am not sure is necessarily a good thing. In any case, we aspire/envision to have a nested plug/unplug, I think it is a good thing. A helper for one subsystem may have its large batch of operations inside plug/unplug pair, another help may do the same, and the caller of these two helpers may want to say plug call helper A A does plug A does many things A does unplug call helper B B does plug B does many things B does unplug unplug to "cancel" the unplug helper A and B has. > In that world, > having a subsystem not maintain a balanced pairing could be a problem. And in such a world, you never want to have end-if-active to implement what you are doing here, as you may end up being not properly nested: begin begin do many things if some condtion end_if_active do more things end end > An alternative API here could be to have an "flush_odb_transaction" > call to make the objects visible at this point. Yes, what you want is a forced-flush instead, I think. So I suspect you'd want these three primitives, perhaps? * begin increments the nesting level - if outermost, you may have to do real "setup" things - otherwise, you may not have anything other than just counting the nesting level * flush implements unplug, fsync, etc. and does so immediately, even when plugged. * end decrements the nesting level - if outermost, you'd do "flush". - otherwise, you may only count the nesting level and do nothing else, but doing "flush" when you realize that you've queued too many is not a bug or a crime.