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

fix: add acquire timeout to sqlite database connection #1590

Merged
merged 2 commits into from Jan 19, 2024
Merged

Conversation

ellie
Copy link
Member

@ellie ellie commented Jan 19, 2024

This should fix #1503

I wasn't able to trigger enough IO pressure for the SQL connection to be a problem.

This adds local_timeout to the client config. This is a float, and represents the number of seconds (units in line with the other timeouts, though those are ints). Users may well want to reduce this if they regularly have issues, but by default I think 2s is fine and avoids a non-responsive system in bad situations.

This should fix #1503

I wasn't able to trigger enough IO pressure for the SQL connection to be
a problem.

This adds `local_timeout` to the client config. This is a float, and
represents the number of seconds (units in line with the other timeouts,
though those are ints). Users may well want to reduce this if they
regularly have issues, but by default I think 2s is fine and avoids a
non-responsive system in bad situations.
@ellie
Copy link
Member Author

ellie commented Jan 19, 2024

Oh also I am still planning on adding an Atuin daemon, which should totally avoid the issue referenced above. Just

  1. That's a MUCH larger change
  2. We'd probably want timeouts there too anyway

@ellie ellie merged commit 8899ce5 into main Jan 19, 2024
8 checks passed
@ellie ellie deleted the ellie/timeout branch January 19, 2024 15:45
@happysalada
Copy link

if you have a moment to make a new release, I'd be happy to update the version on nixos and make another test.
Nixos on rebuilds does a lot of IO, so it will be very easy to check that the problem didn't hide another problem.
Finally, thank you for making this and for all the time you spent on this project!

@ppom0
Copy link

ppom0 commented Jan 21, 2024

Thanks a lot @ellie, it seems great! I'll wait for a release, but no stress about this ofc.

My use case is the same as happysalada, I'm using NixOS too, and its rebuilds are very IO intensive.

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

Successfully merging this pull request may close these issues.

atuin makes the shell unresponsive under high I/O pressure
3 participants