Skip to content

improve: status cache for next reconciliation - only the lock version #2800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
May 16, 2025

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented May 15, 2025

No description provided.

csviri added 9 commits May 14, 2025 09:21
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@openshift-ci openshift-ci bot requested review from metacosm and xstefank May 15, 2025 10:05
@csviri csviri changed the title only lock p cache mprove: status cache for next reconciliation - only the lock version May 15, 2025
@csviri
Copy link
Collaborator Author

csviri commented May 15, 2025

Alternative to: #2799
keeping only the lock version.

@csviri csviri changed the title mprove: status cache for next reconciliation - only the lock version improve: status cache for next reconciliation - only the lock version May 15, 2025
Signed-off-by: Attila Mészáros <[email protected]>
metacosm added 2 commits May 16, 2025 09:05
Signed-off-by: Chris Laprun <[email protected]>
Signed-off-by: Chris Laprun <[email protected]>
.getControllerEventSource()
.handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary);
return updatedResource;
public static <P extends HasMetadata> P editStatusAndCacheResource(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this method should be renamed patchStatusAndCacheResource to make it clearer that it's using a patch operation. I understand the idea to follow the fabric8 naming but in this case, it's just a bad name :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, problem is that already there is a patchStatusAndCacheResource., but maybe jsonPatchStatusAndCacheResource ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok removed the json prefixes, seemed to long

logWarnIfResourceVersionPresent(primary);
return patchAndCacheStatus(
primary, context, () -> context.getClient().resource(primary).editStatus(operation));
public static <P extends HasMetadata> P patchStatusAndCacheResource(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should be renamed mergePatchStatusAndCacheResource, imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe jsonMergePatchStatusAndCacheResource?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok removed the json prefixes, seemed to long

csviri added 2 commits May 16, 2025 10:36
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri csviri requested a review from metacosm May 16, 2025 08:38
csviri added 2 commits May 16, 2025 10:50
Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>
@csviri csviri merged commit 5520c14 into main May 16, 2025
25 checks passed
@csviri csviri deleted the only-lock-p-cache branch May 16, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caching status for followup reconciliation and optimistic locking
2 participants