[ENH] add vacuum CLI command

[codetheweb]

Implementation of the CLI command described in this CIP: #2498. Terminal UI demo:

Screen.Recording.2024-07-24.at.4.48.03.PM.mov

Warning that's logged when automatic pruning is disabled:

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[codetheweb]

rich was already a subdependency of typer, so this is not a new dependency

[codetheweb]

may be useful in the future, but currently only used for assertions in tests

[codetheweb]

VACUUM can be safely run with concurrent users (e.g. the chroma server running) but will block all queries as it takes an exclusive lock

[tazarov]

For very large DBs this can take up to a min or more. There are at least two options here:

• conn.execute('PRAGMA busy_timeout = 5000') - https://www.sqlite.org/c3ref/busy_timeout.html
• using WAL mode, although, this may not play nicely with VACUUM into

In prior implementations I did force the whole connection pool to close all connections and do a global lockout to ensure no incoming requests are processed, but for that to work this command needs to execute within the fastapi process/worker.

[codetheweb]

sorry, not clear to me what your comment is addressing, why would we want to enable WAL mode on SQLite?

I do change the busy_timeout before running VACUUM just so that it's not indefinitely blocked by long-running writes (our global timeout is set to 16m currently).

[codetheweb]

I would like to add a check before vacuuming that there's enough free disk space (> current db size) but this turned out to be fairly difficult:

• SQLite doesn't directly expose its temporary directory (or at least the PRAGMA that does is deprecated)
• how would we set up tests to simulate a mount point running out of space?

If the vacuum does fail because there's not enough space, that error will propagate back to stdout.

[tazarov]

@codetheweb, here are some thoughts:

• You can vacuum into a new file VACUUM into <file> - this mimics copy on write, which I think might be a good fit here as we control where things go and how we do checks. This will not work if the DB is live, though, as files need to be moved/removed.
• As far as testing goes, it could do the trick of restricting space in a docker test.

services:
  chroma:
    image: chroma
    tmpfs:
      - /chroma/chroma:size=100M

[codetheweb]

You can vacuum into a new file VACUUM into - this mimics copy on write, which I think might be a good fit here as we control where things go and how we do checks. This will not work if the DB is live, though, as files need to be moved/removed.

I think the fact that this puts the correctness burden on us makes it a dealbreaker.

In practice I'm not sure if this will be a real issue; if the WAL is pruned prior to VACUUM in theory the amount of temporary space needed should be much less than the current database size; assuming the size of most databases is dominated by the WAL.

[tazarov]

In one of the prior incarnations of this, we also reported on the stats of what the vacuum did, e.g., the size of the SQLite3 file before/after and, optionally, the number of free_pages before/after. This could be good information to help users understand the effects of vacuuming.

[codetheweb]

In one of the prior incarnations of this, we also reported on the stats of what the vacuum did, e.g., the size of the SQLite3 file before/after and, optionally, the number of free_pages before/after. This could be good information to help users understand the effects of vacuuming.

We could expose this in the future, but I don't see this being particularly useful right now as vacuum operations after the initial one aren't expected to free much space. People can always do du -d0 -h {data_dir} to see the disk usage themselves too. :)

[atroyn]

In one of the prior incarnations of this, we also reported on the stats of what the vacuum did, e.g., the size of the SQLite3 file before/after and, optionally, the number of free_pages before/after. This could be good information to help users understand the effects of vacuuming.

We could expose this in the future, but I don't see this being particularly useful right now as vacuum operations after the initial one aren't expected to free much space. People can always do du -d0 -h {data_dir} to see the disk usage themselves too. :)

I do think it's useful to tell users what the very first vacuum did for them, and then if they try doing it again when they don't need to, to see that it does not do much for them.

[codetheweb]

In one of the prior incarnations of this, we also reported on the stats of what the vacuum did, e.g., the size of the SQLite3 file before/after and, optionally, the number of free_pages before/after. This could be good information to help users understand the effects of vacuuming.

We could expose this in the future, but I don't see this being particularly useful right now as vacuum operations after the initial one aren't expected to free much space. People can always do du -d0 -h {data_dir} to see the disk usage themselves too. :)

I do think it's useful to tell users what the very first vacuum did for them, and then if they try doing it again when they don't need to, to see that it does not do much for them.

[codetheweb]

I think we need to hold off on adding a warning on startup if vacuum hasn't been run; that's dependent on storing the auto-pruning setting (if a user inits 0.6 on a fresh data directory, vacuum doesn't need to be run). I'll add it separately to our internal tracker. decided to put this in the same stack instead

[codetheweb]

• [ENH] add vacuum CLI command #2519 👈
• [ENH] automatically clean WAL #2557
• [ENH] add .clean_log() to Producers #2549
• [ENH] simplify logic for when to persist index changes (re-apply with fix) #2545
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @codetheweb and the rest of your teammates on Graphite

[codetheweb]

Merge activity

• Jul 29, 8:18 PM EDT: @codetheweb merged this pull request with Graphite.