cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.21k stars 3.82k forks source link

addsstable: .ingested files should be removed on startup and/or in tmpdir #26813

Open dt opened 6 years ago

dt commented 6 years ago

currently we make links or copies of sideload SSTs with the .ingested suffix during ingest that are expected to be immediately deleted by rocksdb as part of the ingestion process.

In the event of an unfortunately timed crash however, these orphaned links/copies will never be cleaned up (with the links preventing the full deletion of the original raft SST even after truncation).

Since these files should never live longer than the ingest call, I think we can safely delete any *.ingested files from the side load dir during startup. We could also create them in a tmpdir rather than in the sideload dir, as they are not part of the durable raft state.

(capturing discussion with @benesch)

Jira issue: CRDB-4985

Epic CRDB-39898

dt commented 6 years ago

cc @mjibson I'll let you and @tschottdorf figure out if/who should own this one.

tbg commented 6 years ago

We could also create them in a tmpdir rather than in the sideload dir, as they are not part of the durable raft state.

In principle I would like this, but the temp dirs may not be on the same file system as RocksDB, so it's probably not a good idea to change it.

dt commented 6 years ago

may not be on the same file system as RocksDB

I was thinking we'd use one in the store dir, which IIRC isn't supposed to span filesystems.

tbg commented 6 years ago

Oh, yeah, that's fine - I thought you were suggesting one of the sql temp dirs (not really sure how all of this stuff works). Then let's preferably do it like that.

tbg commented 6 years ago

@mjibson assigning you to match the bulkio triage. Doubt the core team will look at this in quite a while.

dt commented 6 years ago

oh, huh, looks like the per-store tempdir we used to have is gone (backup/restore used to use one for sstable creation before we switched to in-mem, but i guess when we switched, we removed the store code than managed it).

dt commented 6 years ago

short-term fix might just be a walk of aux to delete *.ingested

rolandcrosby commented 5 years ago

@tbg is this still relevant or can we close?

tbg commented 5 years ago

Still TODO