Neeraj Singh [off-list ref] writes:
quoted
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.
On Thu, Mar 24, 2022 at 2:34 PM Junio C Hamano [off-list ref] wrote:
Neeraj Singh [off-list ref] writes:
quoted
quoted
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.
I don't love the tentative object store that keeps things invisble,
but that was the safest way to maintain the invariant that no
loose-object name appears in the ODB without durable contents. I
think we want the "durability/ordering boundary" part of database
transactions without necessarily needing full abort/commit semantics.
As you say, we don't need full atomicity, but we do need ordering to
ensure that blobs are durable before trees pointing them, and so on up
the merkle chain. The begin/end pairs help us defer the syncs
required for ordering to the end rather than pessimistically assuming
that every object write is the end.
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.
quoted
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
quoted
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.
Yes, I'll move in this direction. Thanks for the feedback.