Automated backport of #895: Add support for resource syncer to use a shared informer #913: Use informer to retry failed resources due to missing #915
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
|
🤖 Created branch: z_pr915/tpantelis/automated-backport-of-#895-#913-upstream-release-0.17 |
...reduce the informer cache memory usage. Fixes submariner-io#859 Signed-off-by: Tom Pantelis <[email protected]>
This was observed in a unit test elsewhere that configures a resync period. On resync, the K8s DeltaFIFO retrieves every object from the cache store and re-queues them. It also invokes the transform function which results in a data race when the resource syncer tries to nil the ManagedFields. This is because the object instance is the same as that stored in the cache which was previously accessed by another thread. So mutating it without the protection of a lock is unsafe. This is really an issue with the DeltaFIFO - it is documented that the "TransformFunc sees the object before any other actor, and it is now safe to mutate the object in place instead of making a copy" however this is not the case on a re-sync. The DeltaFIFO should either elide the TransformFunc on re-sync or make a copy of the object retrieved from the cache store. As a workaround, the resource syncer should only set ManagedFields to nil if it is non-nil, which will be the case a a re-sync. Added a unit test to cover this case. Also the object passed to the TransformFunc could be a DeletedFinalStateUnknown so we need to handle that as well. Signed-off-by: Tom Pantelis <[email protected]>
Signed-off-by: Tom Pantelis <[email protected]>
tpantelis
force-pushed
the
automated-backport-of-#895-#913-upstream-release-0.17
branch
2 times, most recently
from
ec0b7ac to
d971880
Compare
Some users may want to configure a shared informer rather than use the internal dedicated informer for efficiency if there's multiple consumers of a resource type. Signed-off-by: Tom Pantelis <[email protected]>
...rather than exponential requeuing which is problematic at scale. The user can specify a SharedInformer that is used to requeue resources that had previously failed when a missing namespace is later created. Signed-off-by: Tom Pantelis <[email protected]>
Signed-off-by: Tom Pantelis <[email protected]>
tpantelis
force-pushed
the
automated-backport-of-#895-#913-upstream-release-0.17
branch
from
d971880 to
022fa4b
Compare
dfarrell07
approved these changes
May 16, 2024
aswinsuryan
approved these changes
May 16, 2024
|
🤖 Closed branches: [z_pr915/tpantelis/automated-backport-of-#895-#913-upstream-release-0.17] |
tpantelis
deleted the
automated-backport-of-#895-#913-upstream-release-0.17
branch
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.
Backport of #895 #913 on release-0.17.
#895: Add support for resource syncer to use a shared informer
#913: Use informer to retry failed resources due to missing
For details on the backport process, see the backport requests page.