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

fix(kuma-cp): don't cache filtered data #5574

Merged
merged 6 commits into from
Jan 10, 2023

Conversation

lukidzi
Copy link
Contributor

@lukidzi lukidzi commented Jan 4, 2023

Whenever there is a call to ReadOnlyResourceManager first it hits the cache and later if there is no hit, goes to the store. In case there is a List call that has filters, we filter on the store/pagination store. Later that result is returned, cached, and here we have an issue. The key which keeps the data has no information about filters so the filtered result is stored at the same key as no filtered. Whenever in the code we have a call for List of objects with/without filters we are going to retrieve already cached elements. In the code, I've introduced that the cached store doesn't allow calls with ListByFilterFunc and the cache always has a sorted list.

@lukidzi lukidzi requested review from a team, lahabana and lobkovilya and removed request for a team January 4, 2023 18:36
@lukidzi lukidzi changed the title fix(kuma-cp): don't cache data with filters fix(kuma-cp): don't cache filtered data Jan 4, 2023
@michaelbeaumont
Copy link
Contributor

@lukidzi does this belong in the changelog or is it a

> Changelog: skip

@lukidzi
Copy link
Contributor Author

lukidzi commented Jan 5, 2023

@lukidzi does this belong in the changelog or is it a

> Changelog: skip

It should be in the changelog, that fixes the current wrong behavior

pkg/core/resources/manager/cache.go Show resolved Hide resolved
pkg/core/resources/manager/cache.go Outdated Show resolved Hide resolved
pkg/core/resources/store/options.go Outdated Show resolved Hide resolved
Copy link
Contributor

@lobkovilya lobkovilya left a comment

Choose a reason for hiding this comment

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

LGTM

@lukidzi
Copy link
Contributor Author

lukidzi commented Jan 10, 2023

@Mergifyio backport release-2.0 release-1.8 release-1.7 release-1.6 release-1.5

@mergify
Copy link
Contributor

mergify bot commented Jan 10, 2023

backport release-2.0 release-1.8 release-1.7 release-1.6 release-1.5

✅ Backports have been created

@lukidzi lukidzi merged commit f5b8d76 into kumahq:master Jan 10, 2023
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)

# Conflicts:
#	pkg/xds/sync/ingress_proxy_builder.go
lukidzi added a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 10, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)

# Conflicts:
#	pkg/xds/sync/ingress_proxy_builder.go
Signed-off-by: Lukasz Dziedziak <[email protected]>
mergify bot added a commit that referenced this pull request Jan 11, 2023
* fix(kuma-cp): don't cache filtered data (#5574)

Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fixed parameters

Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fix conflict

Signed-off-by: Lukasz Dziedziak <[email protected]>

Signed-off-by: Lukasz Dziedziak <[email protected]>
Co-authored-by: Łukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 11, 2023
* fix(kuma-cp): don't cache filtered data (#5574)

Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fixed parameters

Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fix conflict

Signed-off-by: Lukasz Dziedziak <[email protected]>

Signed-off-by: Lukasz Dziedziak <[email protected]>
Co-authored-by: Łukasz Dziedziak <[email protected]>
lukidzi added a commit that referenced this pull request Jan 11, 2023
mergify bot added a commit that referenced this pull request Jan 11, 2023
* fix(kuma-cp): don't cache filtered data (#5574)

Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): removed character

Signed-off-by: Lukasz Dziedziak <[email protected]>

Signed-off-by: Lukasz Dziedziak <[email protected]>
Co-authored-by: Łukasz Dziedziak <[email protected]>
mergify bot added a commit that referenced this pull request Jan 11, 2023
* fix(kuma-cp): don't cache filtered data (#5574)

Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fixed parameters

Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fix conflict

Signed-off-by: Lukasz Dziedziak <[email protected]>

* test(kuma-cp): fix wait for goroutine to be done (backport #5638) (#5647)

Signed-off-by: Lukasz Dziedziak <[email protected]>

Signed-off-by: Lukasz Dziedziak <[email protected]>
Co-authored-by: Łukasz Dziedziak <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Jan 11, 2023
* fix(kuma-cp): don't cache filtered data (#5574)

Signed-off-by: Lukasz Dziedziak <[email protected]>
(cherry picked from commit f5b8d76)
Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fixed parameters

Signed-off-by: Lukasz Dziedziak <[email protected]>

* fix(kuma-cp): fix conflict

Signed-off-by: Lukasz Dziedziak <[email protected]>

* test(kuma-cp): fix wait for goroutine to be done (backport #5638) (#5646)

Signed-off-by: Lukasz Dziedziak <[email protected]>

Signed-off-by: Lukasz Dziedziak <[email protected]>
Co-authored-by: Łukasz Dziedziak <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
lukidzi pushed a commit that referenced this pull request Jan 11, 2023
* test(kuma-cp): fix wait for goroutine to be done (backport #5638) (#5648)

Signed-off-by: Lukasz Dziedziak <[email protected]>
lukidzi pushed a commit that referenced this pull request Jan 11, 2023
* test(kuma-cp): fix wait for goroutine to be done (backport #5638) (#5648)

Signed-off-by: Lukasz Dziedziak <[email protected]>
bartsmykla pushed a commit to bartsmykla/kuma that referenced this pull request Jan 14, 2023
Signed-off-by: Lukasz Dziedziak <[email protected]>
Signed-off-by: Bart Smykla <[email protected]>
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.

Recurring log info messages "Another mesh has been added or removed or we must retry"
4 participants