Asynchronous synchronization of Beyla cache #1358
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
mariomac
requested review from
grcevski,
marctc and
rafaelroquetto
as code owners
mariomac
commented
Nov 14, 2024
Comment on lines
-183
to
-190
| select { | ||
| case <-time.After(mp.cfg.SyncTimeout): | ||
| klog().Warn("kubernetes cache has not been synced after timeout. The kubernetes attributes might be incomplete."+ | ||
| " Consider increasing the BEYLA_KUBE_INFORMERS_SYNC_TIMEOUT value", "timeout", mp.cfg.SyncTimeout) | ||
| case err, ok := <-done: | ||
| if ok { | ||
| return nil, fmt.Errorf("failed to initialize Kubernetes informers: %w", err) | ||
| } |
There was a problem hiding this comment.
we didn't realize a bug here.
If the timeout happened, the later returned "informers" variable would be nil, as it requires that the InitInformers function successfully returns from another goroutine.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1358 +/- ##
==========================================
- Coverage 79.06% 72.06% -7.01%
==========================================
Files 145 144 -1
Lines 14648 14661 +13
==========================================
- Hits 11581 10565 -1016
- Misses 2489 3395 +906
- Partials 578 701 +123
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
marctc
approved these changes
Nov 14, 2024
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.
Alleviates the following issues: #1349 and #1354
The Beyla cache service was not accepting connections before it had a complete copy of the Kubernetes cache.
In big clusters (~1000 nodes) that meant that each cache instance would take some minutes to completely initialize, and beyla instances would keep reporting errors.
This PR enables the service as soon as the process start, and keeps accepting Beyla connections while the cache is still synchronizing, making sure that the Beyla clients won't receive the synchronization signal until the cache service is fully synchronized.