Repository navigation
disabled step hold + lost SendCompleted + pruning can permanently de-sync sender & receiver #340
Description
If the recently-added send.step_holds.disable_incremental is set to true (defaults to false) the situation depicated above can lead to sender & receiver getting permanently out of sync, resulting in replication conflict.
Note that step holds protect from this bug because they prevent s4 from being pruned.
Solution 0
Revert send.step_holds.disable_incremental back to its default value false.
Probably not useful to users of send.step_holds.disable_incremental.
Solution 1
Prevent send-side pruning of snapshots younger than the last replication cursor.
This can be achieved through configuration: add a keep: not_replicated rule to the sender.
Probably not useful to users of send.step_holds.disable_incremental.
Solution 2
If send.step_holds.disable_incremental is set, create bookmarks intead of holds.
Create them, like holds, before the replication step starts.
The pruner would prune s4, but there would be an #s4 corresponding to receive-side s4(lrh).
Problem: in an eventual SendCompleted rpc, we would need to move the replication cursor to #s4.
Which we can't unless we have bookmark cloning.
So we'd need to keep those bookmarks around until the replication cursor could be moved at the same guid or a newer snapshot.
Note that the existing but currently dead code for step bookmarks can be used to support this use case:
the purpose of step bookmarks is to be an alternative incremental-send-from if the original incremental-send-from was a user-created bookmark and the snapshot is no longer available.
We don't create them at all ATM, but there is no reason why we shouldn't be creating them if step holds are simply disabled (instead of not being possible because bookmarks cannot be held).
Activity
InsanePrawn commentedon Jun 27, 2020
This relates to a couple thoughts I've had here and there:
a) Smarter pruning grid? What if we had a 'smarter pruning grid' that took holds (that zrepl knows about through the holds cache) into consideration when filling the buckets. (Also account for other pruning rules?) This could cause slight deviations in the intervals of snapshots you're left with after long term pruning depending on replication since then, but it might be worth it to some users. Exact use case
unknownrecovered from faulty bio memory: :)If zrepl were to know about the last sent state, initially read from e.g. a zfs user property on either the dataset or a snapshot, then in-memory, the pruner could be taught not to prune those.
Someone might argue "but the point is not to have on-disk structures": Okay, cool, so you got a mostly always-online setup and want rapid replication, right? Alright, no on-disk status, just connect to the remote on startup, see what's what, local memory after that.
No? Your target machine/pool might be offline for long periods of time? Cool, let's add a way for you to store just that state in a local file. If that's not okay either you either magically expect zrepl to know remote state without storing or connecting anything or you might just be willing to live with the fact that restarting the daemon loses that info, potentially causing 'snapshot loss' as seen in the image above; but you want that performance at the cost of safety, that's why you disabled holds [and bookmarks and properties and the file] in the first place, right?
b) as I said on a voice call initially: add a toggle for holds. Add a toggle for bookmarks. Do not tie them to each other IMO. Might even want to go by abstraction: A toggle for LRHs, a toggle for LSHs, a toggle for LSBs. Pick a combination that makes zrepl useful by default, give the user the choice if they wish to play with them.
c) do you need bookmark cloning? How about a rename? Renaming to the hold should be easy, renames back should be kinda pointless, right? (Even though we could append the old name to the hold_ name and rename back later...) Multi-job setups on the same dataset are not fully covered by this, but while we wait for bookmark cloning to hit all the versions people are running, the other 99% of setups are covered by this.
problame commentedon Jun 27, 2020
I think you are mixing a few things up here. You are talking about renaming holds, but the question starts with bookmarks.
To the best of my knowledge, ZoL doesn't support renaming bookmarks. Newly released FreeBSD 11.4 does though (not the most recent major release 🤷 ).
problame commentedon Jun 27, 2020
That's what
keep: not_replicatedalready does. Before we feed the snapshot list to the (pure) prune rules, we mark those snapshots younger than the replication cursor bookmark as 'not replicated'.problame commentedon Jun 27, 2020
With regards to more config knobs: I think my oversight of the implicit dependency of guaranteed incremental replication on step holds outlined in this issue is a pretty good example why there shouldn't be more knobs unless we a) have more reviewers of the replication protocol or b) formalize the protocol and prove it correct.
I guess I'm simply unable / unwilling to reason about an exponentially growing set of config permutations for zrepl's core component.
Regardless, with regards to this specific issue, the config knob will need to be changed to a tri-state since it really doesn't make any sense to create both step holds and bookmarks
send.incremental.protection=holdscreates step holds, guarantees incrementals + resumability (default)send.incremental.protection=bookmarkscreates step bookmarks, guarantees incrementalssend.incremental.protection=nonedoes not create any step holds / bookmarks, guarantees nothing, leads to this issue[#340] WIP fix it
[#340] WIP fix it
InsanePrawn commentedon Jun 28, 2020
Yep, apparently it was brainfart time.
I was trying to talk about bookmarks, not holds. Derp.
I assumed bookmark renames were a thing and assumed wrong, plus I think you once told me in the past. D'oh!
I was basically talking about step bookmarks or an in-memory version of them + a sort of
keep: stepsrule for best-effort resumability without holds.I'm glad I got you to document it again in here though in case I forget again 🙃
replication + endpoint: replication guarantees: guarantee_{resumabili…
replication + endpoint: replication guarantees: guarantee_{resumabili…
replication + endpoint: replication guarantees: guarantee_{resumabili…