[Bug]: sqlite database corruption on network filesystem AFS

budde25 commented 2 months ago

What did you expect to happen?

The database would remain in a working state on the network filesystem AFS.

What happened?

Pretty regularly the database would get corrupt causing atuin to panic when retrieving history (up arrow). (I don't have an example anymore since I have been using a patched version (a fork) that fixes the issue for a while)

After taking a look on the sqlite docs it looks like this can be mitigated by using a rollback mode (recommendation 3). https://www.sqlite.org/useovernet.html. These include "DELETE", "PERSIST", or "TRUNCATE".

I tried changing the database files from WAL (write-ahead log) to DELETE however the the default behavior of atuin is to overwrite this and change it back to WAL on access.

https://github.com/atuinsh/atuin/blob/ea991d5f453c5ec98f144b1ae1c61857c5096cc8/crates/atuin-client/src/database.rs#L141 https://github.com/atuinsh/atuin/blob/ea991d5f453c5ec98f144b1ae1c61857c5096cc8/crates/atuin-client/src/record/sqlite_store.rs#L44

I tested removing these two lines in a fork that I compiled and manually changed the database files to DELETE and they have been corruption free for a couple weeks so I am fairly confident that did indeed fix my issues. https://github.com/budde25/atuin

It would be great if the database files were not forced onto WAL so that manually changing them would keep them working on AFS or adding an option to the config such as "Network Filesystem Support" that would default them to a rollback mode instead of WAL on access.

Atuin doctor output

{
  "atuin": {
    "version": "18.3.0",
    "sync": null,
    "sqlite_version": "3.44.0"
  },
  "shell": {
    "name": "fish",
    "default": "bash",
    "plugins": [
      "atuin"
    ],
    "preexec": "built-in"
  },
  "system": {
    "os": "Red Hat Enterprise Linux",
    "arch": "x86_64",
    "version": "8.8",
    "disks": [
      {
        "name": "/dev/mapper/rootvg-root",
        "filesystem": "xfs"
      },
      {
        "name": "/dev/mapper/rootvg-tmp",
        "filesystem": "xfs"
      },
      {
        "name": "/dev/sda2",
        "filesystem": "ext4"
      },
      {
        "name": "/dev/mapper/rootvg-usr_vice_cache",
        "filesystem": "xfs"
      },
      {
        "name": "/dev/sda1",
        "filesystem": "vfat"
      },
      {
        "name": "AFS",
        "filesystem": "afs"
      }
    ]
  }
}

Code of Conduct

[X] I agree to follow this project's Code of Conduct

budde25 commented 2 months ago

I would be happy to make a patch to remove the forced change to the mode to WAL on database access (while still creating in WAL mode if file doesn't exist). That would require minimal changes to any code and allow me to mitigate by just manually changing my database files after atuin's initial creation.

If that sounds like a reasonable fix 😄 ?

ellie commented 4 weeks ago

We're unlikely to move away from WAL mode I'm afraid. If you need to patch the database access code to work on a network filesystem, it'll be best for you to maintain that patch separately

I appreciate that your patch would be minimal and provide a nice workaround, however I'd rather be explicit and ensure it's enabled at all times.

WAL provides a whole bunch of benefits in terms of performance and concurrent access. Network filesystems often have issues with SQLite, which can be worked around in some cases and not in others. I'd rather prioritise the safety and perforance of atuin for the vast majority of users in this case.

atuinsh / atuin