Skip to content
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

.Reconcile queue based on resource ID rather than kind #1019

Closed
mjnagel opened this issue Aug 2, 2024 · 1 comment · Fixed by #1025
Closed

.Reconcile queue based on resource ID rather than kind #1019

mjnagel opened this issue Aug 2, 2024 · 1 comment · Fixed by #1025
Assignees
Labels
enhancement New feature or request

Comments

@mjnagel
Copy link
Contributor

mjnagel commented Aug 2, 2024

Is your feature request related to a problem? Please describe.

In uds-core we have a custom resource named Package that we watch for create/update events:

https://github.com/defenseunicorns/uds-core/blob/main/src/pepr/operator/index.ts#L57-L62

Each event is queued via Reconcile. In effect this means that all Package resource creations/updates are placed in a queue together. As one quick example of the effects:

  • Create Package A, B, and C - all in a single apply
  • Result is that nothing happens to Package C until A and B have completed

The same is true on updates:

  • Create Packages A, B, C
  • Wait until all are ready
  • Create Package D and update Package C
  • Package C is not updated until D is fully created

Ideally I would like each Package to be queued separately - so updates on C wait for previous ones to complete, but A/B/D do not affect the timing/waiting.

Describe the solution you'd like

Provide an option for .Reconcile to optionally be queued per resource. This could be done based on resource namespace + name, or some other unique identifier like UID. This would speed up reconcile time when multiple resources are created at the same time.

Describe alternatives you've considered

Separate bindings could be created per namespace or per name, but this does not work well for an extensible operator with resources in arbitrary namespaces.

Additional context

This probably should be an option, but not the only option since certain reconcile code may not behave well on concurrent operations, even if restricted to different resources.

@mjnagel mjnagel added the enhancement New feature or request label Aug 2, 2024
@cmwylie19
Copy link
Collaborator

Spit balling here but we should be able to do with by changing the Queue to Record<string, Queue>, the string would be resourceName/resourceNamespace, so we can distinguish between cluster-scoped objects and namespaced objects. Will start working on this next week

@cmwylie19 cmwylie19 mentioned this issue Aug 3, 2024
5 tasks
@cmwylie19 cmwylie19 self-assigned this Aug 3, 2024
cmwylie19 added a commit that referenced this issue Aug 28, 2024
## Description

Changes the Queue to a queueRecord<string, Queue> so that we enqueue
items of the same name. This creates multiple queues but cuts down on
reconciliation time. This is based on issue #1019.

e2e test is here -
defenseunicorns/pepr-excellent-examples#52 which
includes the original tests

Think about adding a new metrics for queue, maybe items per queue

## Related Issue

Fixes #1019 
<!-- or -->
Relates to #

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Co-authored-by: Barrett LaFrance <barrett.t.lafrance@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants