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(server/settings): write settings to a temp file then move #1067

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Zariel
Copy link

@Zariel Zariel commented Nov 1, 2024

When writing the settings.json file ensure that the file is fully written by writing it to temporary file before renaming it to the final settings path. This should prevent issues where the config gets lost due to the file being corrupted.

Description

When running in K8S both Jellyseerr and Overseerr have suffered from losing the config due to pod restarts, this change makes it so that the settings.json is first wirtten to a temporary file then it is renamed to its final name. This should help prevent cases where a partial or unflushed write cause the loss of the whole settings.

Screenshot (if UI-related)

To-Dos

  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

Issues Fixed or Closed

…d corruption

When writing the settings.json file ensure that the file is fully written by writing it to temporary
file before renaming it to the final settings path. This should prevent issues where the config gets
lost due to the file being corrupted.
Copy link
Collaborator

@gauthier-th gauthier-th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, even if I wouldn't have thought that saving the settings would cause any problems though, because the settings aren't updated very often.
@Fallenbagel wdyt about this?

@Zariel
Copy link
Author

Zariel commented Nov 8, 2024

LGTM, even if I wouldn't have thought that saving the settings would cause any problems though, because the settings aren't updated very often. @Fallenbagel wdyt about this?

I've just moved to Jellyseerr from Overseerr and didn't do a find references but Overseerr was saying settings very frequently. Running in K8s with ceph-block storage I would regularly be losing the settings to the point where I started trying to figure out what to do about it. The only things I can think of is the process being killed mid write or something happening where the kernel buffer doesnt get flushed, ive not been able to nail down the exact sequence of events but I thought would prevent users from being left with corrupted settings which ends up requiring a fresh setup.

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.

2 participants