chore: Update cache library #418
Conversation
| @@ -58,7 +57,7 @@ func (ps projectsService) List(ctx context.Context, name string) (mlp.Projects, | |||
| } | |||
| } | |||
| // Refresh cache and try filtering | |||
| err := ps.refreshProjects(ctx) | |||
| err := ps.refreshProjects(ctx, name) | |||
There was a problem hiding this comment.
When retrieving the projects list, use the name filter if supplied. Without the name filter, all the projects will be retrieved which may be wasteful if we are only interested in one.
|
what happens if the cache keeps increasing? Can it cause OOM? |
@tiopramayudi, that's a good question. Most users set the TTL (using That said, in theory, if there is lots of data added to the cache with a long expiration period, there can be OOM. So we can add a warning to the users when the feature is released, that they should allocate sufficient memory to the transformer containers if they query a lot of data in their transformation steps. |
<!-- Thanks for sending a pull request! Here are some tips for you: 1. Run unit tests and ensure that they are passing 2. If your change introduces any API changes, make sure to update the e2e tests 3. Make sure documentation is updated for your PR! --> **What this PR does / why we need it**: <!-- Explain here the context and why you're making the change. What is the problem you're trying to solve. ---> #418 replaced the in-memory caching library used in the code with `github.com/patrickmn/go-cache` everywhere. However, this could be problematic for the transformer, as indicated by [this comment](#418 (comment)). So, this PR reverts the changes to the caching library for the transformer, so it can continue to rely on `github.com/coocood/freecache`, which can bound the memory allocated for the caching. **Does this PR introduce a user-facing change?**: <!-- If no, just write "NONE" in the release-note block below. If yes, a release note is required. Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information about release notes, see kubernetes' guide here: http://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note Replace Transformer caching library. Support for the transformer env var config CACHE_SIZE_IN_MB is now re-enabled. ``` **Checklist** - [x] Added unit test, integration, and/or e2e tests - [ ] Tested locally - [ ] Updated documentation - [ ] Update Swagger spec if the PR introduce API changes - [ ] Regenerated Golang and Python client if the PR introduce API changes
What this PR does / why we need it:
This PR replaces the use of
"github.com/coocood/freecache"in the codebase with"github.com/patrickmn/go-cache"for the following reasons:freecachelibrary needs us to declare the size of the cache ahead of time.go-cache, on the other hand, is map-based and simpler.freecachelibrary requires each entry to be less than 1/1024 of cache size. This would require us to calculate the cache size ahead of time and does not allow for the occasional large-sized entries.go-cachehas no such restriction.With this change, the transformer config
CACHE_SIZE_IN_MBis no longer required and has been removed.Other changes:
.env.sampleis replaced byenv-config.yaml, based on the changes introduced by #397Does this PR introduce a user-facing change?:
Checklist