Add IOop::Barrier
#1494
Add IOop::Barrier
#1494
Conversation
mkeeter
force-pushed
the
mkeeter/barrier-op
branch
from
56c05b8
to
44d6a40
Compare
mkeeter
force-pushed
the
mkeeter/simplify-complete-jobs
branch
from
9042a37
to
30b2a97
Compare
|
It looks like this just adds the IOop, but the operation won't be submitted until a future PR, and doesn't change replay behavior currently, right? Seems fine to me, as long as that CI failure was a flake and not something real |
mkeeter
force-pushed
the
mkeeter/barrier-op
branch
from
44d6a40
to
7adccc7
Compare
mkeeter
force-pushed
the
mkeeter/barrier-op
branch
from
7adccc7
to
a2f5901
Compare
Yup, this is just adding the |
upstairs/src/client.rs
Outdated
| assert!(read_data.data.is_empty()); | ||
| assert!(extent_info.is_none()); | ||
|
|
||
| if jobs_completed_ok == 2 { |
There was a problem hiding this comment.
For the barrier, do we want jobs_completed_ok == 3?
And, we are not acking this to anyone, but do you plan to use ackable later to indicate that future action can move forward?
There was a problem hiding this comment.
I'm ambivalent! Like you said, we're not actually acking this to anyone; I also don't plan to use presence in ackable_work as an indication for anything here (unlike how it's used in live-repair).
This means the only thing that happens when the job is acked is that it moves from GuestWork::active to GuestWork::completed. Setting the ack to happen at jobs_completed_ok == 3 seems reasonable to me.
There was a problem hiding this comment.
If we are not using ackable for anything, then I would vote for not setting it at all.
Otherwise I'll come back here and see it set and wonder what was consuming that and be confused when I found nothing.
I do care about the dtrace probe though, that one we might actually want to know when this operation has finised on all three downstairs. I don't think I would ever care to know that the barrier had finished on 2/3 downstairs.
There was a problem hiding this comment.
We do need to use ackable, because the job is stored in GuestWork so we need to "ack" it to remove it from that map (even though nothing else happens in the ack).
I changed the threshold to 3 in ed52bf5
mkeeter
force-pushed
the
mkeeter/barrier-op
branch
from
ed52bf5
to
0c2f2df
Compare
|
Ignore the failure in |
(staged on top of #1493)
The
Barrierop does no work, but serves as a full dependency barrier (like a flush) so that the Downstairs can reset its list of completed jobs. Note that it does not updatelast_flushor retire jobs in the Upstairs, because it doesn't actually persist data to disk.This is a building block for removing automatic flushes (see RFD 518).