Stale Cache on Fast Reconcile #74
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/retest |
1 similar comment
|
/retest |
slintes
requested changes
Aug 14, 2023
|
/hold let's consider a better way for dealing with stale cache on quick requeue |
razo7
force-pushed
the
requeue-reconcile-with-offset
branch
from
e8ba7cf
to
5808b39
Compare
razo7
changed the title
After adding a finalizer, and a successful FA we immediately requeue Reconcile. We also log conflict and failing status update and a reason on any updateConditions call
We update this field when any condition value has been changed. Moreover, we can check that field to verify that the status was updated correctly at the end of reconcile by see an offset between their time
razo7
force-pushed
the
requeue-reconcile-with-offset
branch
from
5808b39
to
b64a565
Compare
|
/retest |
slintes
reviewed
Aug 17, 2023
Wait until the cache is updated in order to prevent reading a stale status in the next reconcile and making wrong decisions based on it. We verify that using the LastUpdateTime value of the far object from the beginning of the reconcile and now, the latest version of the CR
razo7
force-pushed
the
requeue-reconcile-with-offset
branch
from
b64a565
to
fbaecd2
Compare
slintes
reviewed
Aug 17, 2023
String can result in anything and contains the unit, which you print yourself already
slintes
approved these changes
Aug 17, 2023
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: razo7, slintes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
|
/retest |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometime status is not updated on a fast reconcile, after a reconcile ends a new reconcile occur immediately (
ctrl.Result{Requeue: true}), due to stale cache. To overcome it we add theLastUpdateTimefield to status so we can use it for comparing far object CR from the beginning of the reconcile and the latest (which is supposed to have the updated status).Similar PR from NHC - medik8s/node-healthcheck-operator#245
ECOPROJECT-1523 ECOPROJECT-1411