feat: bounded cache for informers by csviri · Pull Request #1718 · operator-framework/java-operator-sdk

csviri · 2023-01-13T08:14:17Z

No description provided.

metacosm · 2023-02-20T15:45:12Z

pom.xml

        <directory-maven-plugin.version>1.0</directory-maven-plugin.version>
        <impsort-maven-plugin.version>1.8.0</impsort-maven-plugin.version>
        <okhttp.version>4.10.0</okhttp.version>
+        <caffein.version>3.1.3</caffein.version>


Everything caffeine-related should be named appropriately i.e. caffeine (with an e at the end)

metacosm · 2023-02-20T16:46:20Z

...amework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java

  }
+
+  /**
+   * See <a href=


This needs a better documentation, especially considering that the link doesn't actually provide any information.

metacosm · 2023-02-20T17:48:20Z

...rc/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java

+    synchronized (this) {
+      log.debug("Fetched resource: {}", newRes);
+      if (newRes == null) {
+        existingMinimalResources.remove(key);


Shouldn't we also remove the associated object from the cache?

will revisit this, came to my mind that there an case, where the current impl might be a problem of this method

made an update for this, to be absolutely safe

metacosm · 2023-02-20T17:53:09Z

...ava/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java

+  }
+
+  /**
+   * This would not be needed


Could you elaborate, please?

this was a leftover, removed.

metacosm · 2023-02-21T12:02:46Z

...rc/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/BoundedItemStore.java

+        // If received we just return. Since the resource from informer should be always leading,
+        // even if the fetched resource is null, this will be eventually received as an event.
+        if (actual == null) {
+          existingMinimalResources.remove(key);


I still think that actual should be removed from cache as well.

why?
its checked 2 lines above if it's there, and that operation is synchronized with other operation for that cache. It is not possible to put that item into the cache meanwhile.

...amework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java

sonarqubecloud · 2023-02-21T15:06:22Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

38.9% Coverage
0.0% Duplication

* feat: bounded cache for informers (#1718) * fix: typo caffein -> caffeine (#1795) * feat: now possible to only output non-resource related metrics Fixes #1812. * refactor: extract abstract test fixture to add tests with variations * fix: add missing annotation * tests: add more test variations * fix: make operator non-static so it's registered once per test subclass * feat: introduce builder for MicrometerMetrics, fix test * fix: exclude more tags when not collecting per resource * fix: registry should be per-instance to ensure test independence * fix: make sure we wait a little to ensure event is properly processed * fix: make things work on Java 11, format * fix: also clean metrics on finalizer removal This is needed because the finalizer will trigger a reconciliation that adds a resource-specific metric. * fix: format * refactor: extract common tags Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com> * feat: make per-resource collecting finer-grained We now still collect GVK information when per-resource collection is switched off. * fix: do not create tag for group if not present * fix: remove unreliable no-delay implementation, defaulting to 1s delay * refactor: renamed & documented factory methods to make things clearer * docs: updated metrics section for code changes * feat: avoid emitting tag on empty value * docs: update * fix: format [skip ci] * refactor: use Tag more directly, avoid unneeded work, use constants * fix: change will happen instead of might * docs: add missing timer Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com> * docs: fix wrong & missing information * refactor: add constants * fix: wording [skip ci] Co-authored-by: Attila Mészáros <csviri@gmail.com> --------- Co-authored-by: Attila Mészáros <csviri@gmail.com> Co-authored-by: Sébastien CROCQUESEL <88554524+scrocquesel@users.noreply.github.com>

csviri self-assigned this Jan 13, 2023

csviri force-pushed the next branch from 45baa8c to 34f3e0f Compare January 19, 2023 09:23

csviri force-pushed the bounded-cache-item-store branch from c4ffb56 to 1dbe752 Compare January 19, 2023 09:23

csviri force-pushed the next branch 2 times, most recently from 5aefd80 to 4af2cc5 Compare January 30, 2023 14:38

csviri force-pushed the bounded-cache-item-store branch from a35083d to fd65df4 Compare January 31, 2023 12:57

csviri force-pushed the next branch 3 times, most recently from fe820ca to 55f5d0b Compare February 8, 2023 13:50

csviri force-pushed the bounded-cache-item-store branch from fd65df4 to dba990f Compare February 10, 2023 08:31

csviri changed the title ~~feat: bounded cache (variant 2)~~ feat: bounded cache for informers Feb 13, 2023

csviri mentioned this pull request Feb 13, 2023

Extend bounded caches for External Resources #1767

Open

csviri force-pushed the next branch from 559afa9 to 0d2b10a Compare February 14, 2023 15:52

csviri force-pushed the bounded-cache-item-store branch from c03cd5a to 63c30ed Compare February 15, 2023 08:25

csviri marked this pull request as ready for review February 15, 2023 13:25

csviri requested a review from metacosm February 15, 2023 13:26

metacosm requested changes Feb 20, 2023

View reviewed changes

metacosm force-pushed the bounded-cache-item-store branch from 8c801ae to c4647a1 Compare February 20, 2023 20:11

metacosm approved these changes Feb 21, 2023

View reviewed changes

metacosm reviewed Feb 21, 2023

View reviewed changes

...amework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java Show resolved Hide resolved

metacosm reviewed Feb 21, 2023

View reviewed changes

...amework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java Show resolved Hide resolved

csviri force-pushed the bounded-cache-item-store branch from 08caef2 to 226be1e Compare February 21, 2023 12:24

csviri added 7 commits February 21, 2023 15:08

feat: bounded cache (variant 2)

40f6ae1

wip

e267cd8

naming

b0ca354

added cache to configs

0b82aac

using new item store

f2c83db

wip

329e071

working bounded cache

2a51e5a

metacosm and others added 7 commits February 21, 2023 15:08

refactor: make constructors more coherent

45ce799

refactor: do not look up constructor on each call

7163533

fix: rename methods more appropriately

e21febf

naming fixes

092aa97

docs

760f57d

fixes

78c83f6

additional condition

50e351a

csviri force-pushed the bounded-cache-item-store branch from 226be1e to 50e351a Compare February 21, 2023 14:08

csviri added 2 commits February 21, 2023 15:49

fix on return actual

139b27d

impl fix

c3de33b

csviri merged commit 19b91a5 into next Feb 21, 2023

csviri deleted the bounded-cache-item-store branch February 21, 2023 15:31

csviri linked an issue Feb 21, 2023 that may be closed by this pull request

Limited Cache Sizes #886

Closed

csviri added a commit that referenced this pull request Feb 23, 2023

feat: bounded cache for informers (#1718)

1222090

csviri added a commit that referenced this pull request Feb 28, 2023

feat: bounded cache for informers (#1718)

f7ab5b2

csviri added a commit that referenced this pull request Mar 3, 2023

feat: bounded cache for informers (#1718)

b7e9ed5

metacosm mentioned this pull request Mar 9, 2023

feat: remove CR meters when they are deleted (after a delay) #1805

Merged

csviri added a commit that referenced this pull request Mar 10, 2023

feat: bounded cache for informers (#1718)

0fee696

csviri added a commit that referenced this pull request Mar 14, 2023

feat: bounded cache for informers (#1718)

8600578

csviri added a commit that referenced this pull request Mar 15, 2023

feat: bounded cache for informers (#1718)

93c6096

metacosm pushed a commit that referenced this pull request Mar 22, 2023

feat: bounded cache for informers (#1718)

11e21d6

metacosm pushed a commit that referenced this pull request Mar 27, 2023

feat: bounded cache for informers (#1718)

c6e10aa

metacosm pushed a commit that referenced this pull request Mar 27, 2023

feat: bounded cache for informers (#1718)

0cbe517

csviri added a commit that referenced this pull request Mar 27, 2023

feat: bounded cache for informers (#1718)

5098ffc

metacosm pushed a commit that referenced this pull request Mar 27, 2023

feat: bounded cache for informers (#1718)

4e5794f

metacosm pushed a commit that referenced this pull request Mar 27, 2023

feat: bounded cache for informers (#1718)

47719f7

csviri added a commit that referenced this pull request Mar 28, 2023

feat: bounded cache for informers (#1718)

ef37520

csviri added a commit that referenced this pull request Mar 28, 2023

feat: bounded cache for informers (#1718)

75b9208

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: bounded cache for informers#1718

feat: bounded cache for informers#1718
csviri merged 27 commits intonextfrom
bounded-cache-item-store

csviri commented Jan 13, 2023

Uh oh!

metacosm Feb 20, 2023

Uh oh!

metacosm Feb 20, 2023

Uh oh!

csviri Feb 21, 2023

Uh oh!

metacosm Feb 20, 2023

Uh oh!

csviri Feb 21, 2023

Uh oh!

csviri Feb 21, 2023

Uh oh!

metacosm Feb 20, 2023

Uh oh!

csviri Feb 21, 2023

Uh oh!

metacosm Feb 21, 2023

Uh oh!

csviri Feb 21, 2023

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

csviri commented Jan 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants