feat: support multiple sync sources #2852
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
julianKatz
reviewed
Jun 29, 2023
julianKatz
reviewed
Jul 6, 2023
acpana
force-pushed
the
acpana/cmt-replay-6-r-clean
branch
from
447c40d to
57ca58d
Compare
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
dirty, refactor: make cm watch aware Signed-off-by: Alex Pana <[email protected]> dirty: frontload with gvk aggregator Signed-off-by: Alex Pana <[email protected]> dirty: use gvk aggregator for config Signed-off-by: Alex Pana <[email protected]> dirty, refactor: replayData is async Signed-off-by: Alex Pana <[email protected]> dirty, refactor: wipe and replay are async Signed-off-by: Alex Pana <[email protected]> dirty, refactor: dual controllers Signed-off-by: Alex Pana <[email protected]> dirty, refactor: nil check props Signed-off-by: Alex Pana <[email protected]> dirty, refactor: cm.Start Signed-off-by: Alex Pana <[email protected]> dirty, fix: mark gvks to sync Signed-off-by: Alex Pana <[email protected]> tests Signed-off-by: Alex Pana <[email protected]>
dirty, squash: config_c changes Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
acpana
force-pushed
the
acpana/cmt-replay-6-r-clean
branch
from
57ca58d to
21b92e9
Compare
maxsmythe
reviewed
Jul 6, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
maxsmythe
reviewed
Jul 7, 2023
|
@anlandu FYI |
Signed-off-by: Alex Pana <[email protected]>
julianKatz
reviewed
Aug 18, 2023
julianKatz
reviewed
Aug 18, 2023
julianKatz
reviewed
Aug 18, 2023
julianKatz
reviewed
Aug 18, 2023
julianKatz
reviewed
Aug 18, 2023
julianKatz
reviewed
Aug 18, 2023
Signed-off-by: Alex Pana <[email protected]>
maxsmythe
reviewed
Aug 23, 2023
Signed-off-by: Alex Pana <[email protected]>
Signed-off-by: Alex Pana <[email protected]>
maxsmythe
approved these changes
Aug 24, 2023
|
✅ |
Signed-off-by: Alex Pana <[email protected]>
sozercan
approved these changes
Aug 29, 2023
AFAICT, the root cause is an internal controller race in config_c that triggers an additional reconcile request to be sent down the previous wrapped channel which will block as it's not being read. Signed-off-by: Alex Pana <[email protected]>
ritazh
reviewed
Aug 30, 2023
| c.needToList = false | ||
| relistStopChan = make(chan struct{}) | ||
|
|
||
| go c.replayGVKs(ctx, gvksToRelist, relistStopChan) |
There was a problem hiding this comment.
This for loop can be spawning a lot of new goroutines. If the new goroutine blocks indefinitely or takes a long time to execute, you could run into a goroutine leak and/or lock contention. is this a concern?
There was a problem hiding this comment.
no error handling for c.replayGVKs ?
There was a problem hiding this comment.
the goroutine should stop when the relistStopChan is closed or ctx is Done. For now, we log the errors if there's a replay issue with a GVK but we also have a retry mechanism in the func to protect against transient errors.
There was a problem hiding this comment.
I'm going to merge this in for now but please feel free to reach out if things don't seem quite right.
ritazh
reviewed
Aug 30, 2023
|
|
||
| // stop any goroutines that were relisting before | ||
| // as we may no longer be interested in those gvks | ||
| close(relistStopChan) |
There was a problem hiding this comment.
we are closing relistStopChan without checking whether it has already been closed, which could result in a panic.
There was a problem hiding this comment.
the invariant here should be that this codesite is the only place that opens/ closes the channel here so we should be good IIRC.
This was referenced Aug 31, 2023
salaxander
pushed a commit
to salaxander/gatekeeper
that referenced
this pull request
Sep 13, 2023
Signed-off-by: Alex Pana <[email protected]>
Mattes83
pushed a commit
to Mattes83/gatekeeper
that referenced
this pull request
Oct 25, 2023
Signed-off-by: Alex Pana <[email protected]>
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.
This patch introduces a
CacheManagertype to manage any constraint framework data caching needs.