reconciler: Add WaitUntilReconciled method #137
Conversation
reconciler/types.go
Outdated
| // table changes up to untilRevision. Returns ctx.Err() if the context | ||
| // is cancelled. | ||
| // Note: errors from Update/Delete are treated as reconciled. | ||
| WaitUntilReconciled(ctx context.Context, untilRevision statedb.Revision) error |
There was a problem hiding this comment.
Do we have a use-case for waiting for objects to be reconciled successfully (e.g. all marked StatusDone) until a given revision?
It would basically require a) wait for the reconciler to catch up to a given revision and b) wait until there's nothing in the retry queue whose "original revision" is lower than untilRevision. Much more complicated what is being done here so would be nice to not have to support that.
There was a problem hiding this comment.
As opposed to ignoring errors you mean? The main use case for for WaitUntilReconciled that I had in the past was always with regards to a BPF map. The goal being to ensure that the contents of the StateDB table has been pushed down to the BPF map and is visible to a BPF program.
So if there were transient errors (that are expected to resolve quickly), then yeah, in such a case it would be "more correct" to wait for those transient errors to resolve before continuing. I'm not sure if our current bpf.NewMapOps implementation can return transient errors, or if all errors are basically persistent.
For persistent errors (that can basically only be resolved by retracting the update), I don't think there's much point in trying to wait for them. I would expect callers of an error-aware WaitUntilReconciled to time out in such a case anyway.
Having said that, just waiting for nothing to be pending anymore for a certain revision is already a big improvement over the status quo, so I'm already happy with this as is.
|
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
8e11846 to
833ab84
Compare
|
I looked again at the code that spawned my original issue #58 One that that is particular to that code:
I think it would be interesting the progress tracker also exposes a method to get the reconciled revision. It could be as simple as returning the current revision in |
Ah yeah good idea. I'll add the current revision to the return value. |
|
Another idea that just spawned while looking at my client code: While it is hard to handle |
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
833ab84 to
96fb77d
Compare
Ended up implementing this. It required some changes we'll need to think carefully about: it split the committing of results into two write transactions, one to commit the normal incremental results and another one to commit retry results. This was needed since we only enqueue retries after we've done the |
Let's raise the minimum required Go version for the next minor StateDB release. This will allow using 'synctest' in the tests here. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
If we use 'synctest' then we can't close channels outside the synctest bubble. The Changes() method creates a `*changeIterator` and registers a finalizer for it that unregisters the delete tracker from the table. As the delete trackers are stored in a 'part.Map` there's a watch channel that gets closed and this triggers a panic if this happens in a synctest. To avoid this issue add a 'Close()' method to the 'ChangeIterator' interface to allow optionally closing the iterator and avoiding the finalizer. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Add `Reconciler.WaitUntilReconciled` method to allow waiting for the
reconciler to catch up processing to a given revision.
Example usage:
```
wtxn := db.WriteTxn(table)
table.Insert(wtxn, &Obj{ID: 1, Status: reconciler.StatusPending()})
table.Insert(wtxn, &Obj{ID: 2, Status: reconciler.StatusPending()})
table.Insert(wtxn, &Obj{ID: 3, Status: reconciler.StatusPending()})
revToWaitFor := table.Revision(wtxn)
wtxn.Commit()
// Block until reconciler has catched up to [revToWaitFor] or [ctx]
// is cancelled.
myReconciler.WaitUntilReconciled(ctx, revToWaitFor)
```
Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
These were never used by Cilium and they are very inefficient way of waiting for objects to be reconciled. Just drop these as we now have [Reconciler.WaitUntilReconciled]. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
4f14e49 to
de07298
Compare
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
de07298 to
622c12a
Compare
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
622c12a to
13d54f0
Compare
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
13d54f0 to
ea9c4e5
Compare
Extend [Reconciler.WaitUntilReconciled] to also indicate whether retries are pending for any objects with a revision below or equal to [untilRevision]. The committing of results is split into two: one after normal incremental processing of pending objects and one after processing retries. This way the entries that failed to reconcile are pushed to the retry queue and we can check the low watermark to produce 'retriesPending'. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
joamaki
force-pushed
the
pr/joamaki/wait-until-reconciled
branch
from
ea9c4e5 to
0886cb6
Compare
gandro
left a comment
gandro
left a comment
There was a problem hiding this comment.
I lack understanding for the more subtle internals of the reconciler, so I can't vouch for the priority queue changes. But the API now looks very usable to me! Thanks for tackling this
4 participants
Add
Reconciler.WaitUntilReconciledmethod to allow waiting for the reconciler to catch up processing to a given revision.Example usage: