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

Properly clear TResources and fix race condition #358

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

BenB196
Copy link
Contributor

@BenB196 BenB196 commented Jun 17, 2023

This change ensures that the proper resource type gets cleared during timeout/WatcherClosed.

Previously, it was always calling against V1Secret, this caused it to clear the V1Secret ConcurrentDictionary(ies) 3 times.

2023-06-17 10:42:51.343 -04:00 [INF] (ES.Kubernetes.Reflector.Core.SecretWatcher) Session closed. Duration: 00:00:15.1274168. Faulted: False.
2023-06-17 10:42:51.343 -04:00 [INF] (ES.Kubernetes.Reflector.Core.ConfigMapWatcher) Session closed. Duration: 00:00:15.1206268. Faulted: False.
2023-06-17 10:42:51.343 -04:00 [INF] (ES.Kubernetes.Reflector.Core.NamespaceWatcher) Session closed. Duration: 00:00:15.2151075. Faulted: False.
2023-06-17 10:42:51.347 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources
2023-06-17 10:42:51.347 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources
2023-06-17 10:42:51.348 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources

In some cases this could also lead to a race condition, where the actual V1Secret clears and regenerates, but then gets cleared again by one of the other threads closing.

2023-06-17 10:43:06.359 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources
2023-06-17 10:43:06.359 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources
2023-06-17 10:43:06.360 -04:00 [INF] (ES.Kubernetes.Reflector.Core.SecretWatcher) Requesting V1Secret resources
2023-06-17 10:43:06.361 -04:00 [INF] (ES.Kubernetes.Reflector.Core.NamespaceWatcher) Session closed. Duration: 00:00:15.0102368. Faulted: False.
2023-06-17 10:43:06.362 -04:00 [INF] (ES.Kubernetes.Reflector.Core.ConfigMapWatcher) Requesting V1ConfigMap resources
2023-06-17 10:43:06.365 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources

This change also adds a new debug log for WatcherClosed that I found to be helpful when troubleshooting this issue.

Related to: #337

After making this change, we can now see that V1Secret only gets cleared once, and now V1ConfigMap gets cleared, and nothing runs for V1Namespace as defined by the if statement

2023-06-17 10:52:28.125 -04:00 [INF] (ES.Kubernetes.Reflector.Core.SecretWatcher) Session closed. Duration: 00:00:15.1385409. Faulted: False.
2023-06-17 10:52:28.125 -04:00 [INF] (ES.Kubernetes.Reflector.Core.ConfigMapWatcher) Session closed. Duration: 00:00:15.1308554. Faulted: False.
2023-06-17 10:52:28.125 -04:00 [INF] (ES.Kubernetes.Reflector.Core.NamespaceWatcher) Session closed. Duration: 00:00:15.2297132. Faulted: False.
2023-06-17 10:52:28.128 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.SecretMirror) Cleared sources for V1Secret resources
2023-06-17 10:52:28.128 -04:00 [DBG] (ES.Kubernetes.Reflector.Core.ConfigMapMirror) Cleared sources for V1ConfigMap resources
2023-06-17 10:52:28.129 -04:00 [INF] (ES.Kubernetes.Reflector.Core.NamespaceWatcher) Requesting V1Namespace resources
2023-06-17 10:52:28.130 -04:00 [INF] (ES.Kubernetes.Reflector.Core.SecretWatcher) Requesting V1Secret resources
2023-06-17 10:52:28.131 -04:00 [INF] (ES.Kubernetes.Reflector.Core.ConfigMapWatcher) Requesting V1ConfigMap resources

@Nidafjoll
Copy link

Any timeline when this will be merged? Having the same issue mentioned in #337

Copy link
Contributor

@winromulus winromulus left a comment

Choose a reason for hiding this comment

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

LGTM

@winromulus
Copy link
Contributor

Thank you @BenB196 for you contribution on this!

@winromulus winromulus merged commit 7f6be79 into emberstack:main Jun 26, 2023
2 checks passed
@BenB196 BenB196 deleted the patch-clear-resource-condition branch June 26, 2023 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants