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

allow option to not cache on non-zero exit code #25

Open
dtanner opened this issue Mar 25, 2021 · 7 comments
Open

allow option to not cache on non-zero exit code #25

dtanner opened this issue Mar 25, 2021 · 7 comments

Comments

@dtanner
Copy link

dtanner commented Mar 25, 2021

Sometimes the expensive command I run fails. e.g. A brief network connectivity issue that I notice and fix after running the command. The problem when using bc::cache is that the result is cached. I'm not aware of a normal use scenario where I'd want the result to be cached on non-zero exit code, so that's my first suggestion.
If changing the default behavior isn't desired, it would be nice to allow the option to not cache on non-zero exit code.
p.s. I tried the memoize function, but it doesn't work for my scenario.

Thoughts?

@dimo414
Copy link
Owner

dimo414 commented Mar 27, 2021

Thanks for the FR, this is definitely a reasonable use-case. I'll have to think more about whether this makes sense to add and how to best enable per-invocation cache invalidation/rejection.

What I would suggest instead is implementing the retry logic within your cached function, e.g. as a simple until loop. This would allow you to control the precise retry behavior (such as exponential backoff) as well exactly as what gets output.

Consider also tuning the TTL or REFRESH; in particular reducing the REFRESH rate would minimize the amount of time a bad request is cached, while still using cached data when available, and ensuring a consistent and capped QPS to the backing function.

I'm not aware of a normal use scenario where I'd want the result to be cached on non-zero exit code

Plenty of commands return non-zero in meaningful situations, e.g. grep returns non-zero if no matches are found. Philosophically, bash-cache intends to provide a high-fidelity cache that preserves stderr and the exit code, which may not always be the desired behavior but is consistent and easy to reason about.

A brief network connectivity issue

This might not be an issue in your case, but it's worth calling out that you may actually want to cache such temporary failures in order to avoid cascading issues. If clients stop caching when they see a failure they will all be sending additional load to the backend right when it's become unhealthy, potentially making a bad problem worse.

I tried the memoize function, but it doesn't work for my scenario.

Not too surprising :) I'm debating removing it outright.

@dtanner
Copy link
Author

dtanner commented Mar 28, 2021

Thanks for the detailed response. A little more details about my use case - I have a small-ish utility script, with a function that makes some network calls that require a VPN connection. Sometimes I'm an idiot and forget to enable the VPN, so the cached response is basically garbage that I need to fix immediately. What I've done as a workaround is check the exit code of the call, and on non-zero I remove the cache. (The entire cache directory which is configured to be local to the directory of the script), which would be bad if I used it for more than one thing, but in this case gets me the desired behavior.

@dimo414
Copy link
Owner

dimo414 commented Mar 29, 2021

Hah that's an effective workaround :) I could imagine adding a bc::invalidate_all::[function] operation to limit the invalidation to a particular function, but as you say using a custom cache directory for the script has roughly the same effect.

Thanks again for the FR, I will think further about how to support this better.

@dimo414
Copy link
Owner

dimo414 commented Nov 19, 2021

As an update here, I've been working on bkt, a standalone binary that's spiritually similar to bash-cache but intended for subprocesses instead of shell functions. I just added a --discard-failures flag which should do what you want. If you'd like to try it out I'd welcome any feedback.

I'll keep this open for now because it's still a valid FR for bash-cache, but it's not on my immediate radar especially now that bkt supports this.

@dtanner
Copy link
Author

dtanner commented Nov 19, 2021

cool thanks @dimo414 !

@kumy
Copy link
Contributor

kumy commented Oct 28, 2022

Hah that's an effective workaround :) I could imagine adding a bc::invalidate_all::[function] operation to limit the invalidation to a particular function

Looks similar to the idea there with bc::expire::FUNCTION_NAME

@dimo414
Copy link
Owner

dimo414 commented Oct 28, 2022

Note that bc::invalidate_all would (presumably) behave differently than bc::expire - the former would clear all invocations of that function while the latter would only expire that particular invocation (e.g. bc::expire::foo bar would not invalidate foo baz). I'm not really thrilled about either option, partially because of that potential for confusion.

I am open to providing a way to ignore failed invocations, but the more bells and whistles you want in your caching layer the more I'd encourage you to look into bkt. In addition to simply being harder to extend, bash-cache is designed to be a transparent caching layer whereas bkt is intentionally explicit at the call site. IMO the more we customize the caching behavior the more important it is for it to be explicit.

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

No branches or pull requests

3 participants