Add Function Cache Refresh If Invoker Is Running Container For Function #5327
Conversation
This file contains hidden or 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5327 +/- ##
==========================================
- Coverage 81.08% 76.35% -4.74%
==========================================
Files 239 239
Lines 14259 14271 +12
Branches 575 584 +9
==========================================
- Hits 11562 10896 -666
- Misses 2697 3375 +678 ☔ View full report in Codecov by Sentry. |
ningyougang
reviewed
Sep 28, 2022
| whenUnhandled { | ||
| case Event(PingCache, data: WarmData) => | ||
| val actionId = data.action.fullyQualifiedName(false).toDocId.asDocInfo(data.revision) | ||
| get(entityStore, actionId.id, actionId.rev, true).map(_ => { |
There was a problem hiding this comment.
Improve the performance? to sync the action data from Couchdb with interval time.
ningyougang
approved these changes
Sep 28, 2022
msciabarra
pushed a commit
to nuvolaris/openwhisk
that referenced
this pull request
Nov 23, 2022
…on (apache#5327) * wip * wip * add cache refresh to container proxy Co-authored-by: Brendan Doyle <[email protected]>
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.
Description
This adds a periodic message to the container proxy that will make a request to the function cache to keep the function in the cache. The cache used to store downloaded functions has a keepalive for 5 minutes between accesses before it evicts. The default openwhisk container idleTimeout is 10 minutes. This means if you're using the default, you have a five minute gap between when a function is evicted and the container continues to run where once the container is unpaused it has to redownload the function before serving an activation. This doesn't really make any sense since behavior wise since the warm container already has the data it needs and the function check is mainly just for verifying it hasn't been updated. So this takes re-downloading the function cache off the critical path for an already warm container, but also since the cache is pinged on a 1 minute interval, so less than the cache eviction time; the function should never need to be re-downloaded so long as a container still exists for the function on the invoker.
Heuristically, this makes a noticeable difference to the tp99 cases for functions. Especially functions that are >10mb in size and are infrequently accessed to hit this case.
Related issue and scope
My changes affect the following components
Types of changes
Checklist: