feature: coalescing calls + feature: max cache size #877
Conversation
|
Up for discussion: If you don't consider coalescing a breaking change, I could modify the existing cache behavior instead. The main difference is: cache will only cache successful calls and only at the end of the call. Coalescing will cache both and at the beginning of the call. Also, I'm uncertain external cache transports will work with caching promises necessary for coalescing. I do see current cache is also caching the promise, but I'm currently not using the cache feature of circuitbreaker so I don't know the details and caveats. Probably all memory cache functions will work as expected, but external key/value storage like Redis would require custom implementation and might break coalescing. |
|
would you mind rebasing? i had to add a fix for the CI to pass |
If options.coalesce is set, multiple calls to the circuitbreaker will be handled as one, within the given timeframe (options.coalesceTTL). feature: max cache size To prevent internal cache from growing without bounds (and triggering OOM crashes), the cache is now limited to 2^24 items. This is an insane amount, so options.cacheSize and options.coalesceSize have been added to control this.
|
Hi @lholmquist the branch is up to date with main, can I do anything else to help this PR along? (Or should I just be more patient ;)? ). Thanks. |
Totally my fault for slacking at looking at this 🤣 |
Pull Request Test Coverage Report for Build 11344548094Details
💛 - Coveralls |
|
@daan944 thanks for the contribution, i think this looks pretty good. This will probably be a minor release since it doesn't break any existing functionality |
|
just released as 8.2.0 on npm |
|
Awesome! thanks :) I hope to issue a PR for the types at https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/opossum tomorrow. |
|
I see there's one issue with coalesce: the cachekey is only generated when the 'normal' cache is enabled too. Am working on a fix and will create new PR. |
To not break existing functionality, I chose to not modify the cache behavior but introduce a second one.
If options.coalesce is set, multiple calls to the circuitbreaker will be handled as one, within the given timeframe (options.coalesceTTL).
If true, this provides coalescing of requests to this breaker, in other words: the promise will be cached.
Only one action (with same cache key) is executed at a time, and the other pending actions wait for the result. Performance will improve when rapidly firing the circuitbreaker with the same request, especially on a slower action (e.g. multiple end-users fetching same data from remote). Will use internal cache only. Can be used in combination with options.cache.
Additional fix/feature:
feature: max cache size
To prevent internal cache from growing without bounds (and triggering OOM crashes), the cache is now limited to 2^24 items. This is an insane amount, so options.cacheSize and options.coalesceSize have been added to control this.