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

sql: allow multiple informers to start in parallel #99

Merged
merged 6 commits into from
Sep 23, 2024

Conversation

moio
Copy link
Contributor

@moio moio commented Aug 30, 2024

Creating an Informer used to block creation of any other Informers for the whole duration of the warm-up time via one mutex.

Warm-up can be long if very many resources are involved, and the behavior blocked any unrelated Steve API calls (eg. UI).

This patch allows for multiple Informers to be created in parallel, with one mutex per gvk.

One overall RWMutex is still kept to avoid races with Reset().

best reviewed commit-by-commit

QA notes

This patch allows for the parallelization of the creation of informers and can be verified by looking for "[DEBUG] CacheFor" log messages in rancher debug logs while navigating the UI.

In the example below, note how several Informers start at 9:30:02 and finish at different points in time, with different durations (ConfigMaps is the slowest as it was pre-loaded with 5000 objects).

image

@moio moio requested a review from a team as a code owner August 30, 2024 15:20
@moio moio requested a review from tomleb August 30, 2024 15:21
Copy link
Contributor

@tomleb tomleb left a comment

Choose a reason for hiding this comment

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

That's a nice addition. Some comments/requests for changes.

pkg/cache/sql/informer/factory/informer_factory.go Outdated Show resolved Hide resolved
pkg/cache/sql/informer/factory/informer_factory.go Outdated Show resolved Hide resolved
pkg/cache/sql/informer/factory/informer_factory.go Outdated Show resolved Hide resolved
@moio moio force-pushed the informer_creation_parallelism branch 2 times, most recently from 36edbf4 to 39899ce Compare September 20, 2024 08:12
Creating an Informer used to block creation of any other Informers
for the whole duration of the warm-up time via one mutex.

This patch allows for multiple Informers to be created in parallel,
with one mutex per gvk.

One overall RWMutex is still kept to avoid races with Reset().

Signed-off-by: Silvio Moioli <[email protected]>
Signed-off-by: Silvio Moioli <[email protected]>
@moio moio force-pushed the informer_creation_parallelism branch 2 times, most recently from 4a8a2d8 to c58211e Compare September 20, 2024 09:42
moio and others added 2 commits September 20, 2024 11:46
Co-authored-by: Tom Lebreux <[email protected]>
Signed-off-by: Silvio Moioli <[email protected]>
Signed-off-by: Silvio Moioli <[email protected]>
@moio moio force-pushed the informer_creation_parallelism branch from c58211e to 2c2f081 Compare September 20, 2024 09:46
Copy link
Contributor

@tomleb tomleb left a comment

Choose a reason for hiding this comment

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

One comment on test, otherwise looks good. thanks!

pkg/cache/sql/informer/factory/informer_factory_test.go Outdated Show resolved Hide resolved
Signed-off-by: Silvio Moioli <[email protected]>
@moio moio requested a review from tomleb September 20, 2024 20:50
Copy link
Contributor

@MbolotSuse MbolotSuse left a comment

Choose a reason for hiding this comment

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

Approving to unblock merging (since the required 2 approvals have been made).

@moio moio merged commit ae858d0 into rancher:master Sep 23, 2024
1 check passed
moio added a commit to moio/rancher that referenced this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants