Support ingesting SST files generated by a live DB #12750
Conversation
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| // - ignore cf_id mismatch between cf_id in the files and the CF they are | ||
| // being ingested into. | ||
| // Warning: This ONLY works for SST files where all keys have sequence number | ||
| // zero and with no duplicated user keys (this should be guaranteed if the |
There was a problem hiding this comment.
Not something in this PR, but maybe we can add a boolean flag in table property to indicate if it has duplicate user keys. This can help make it easier to do a sanity check at the ingestion time.
There was a problem hiding this comment.
Yes, I'm considering adding largest seqno to table property. For no duplicate user key but with non-zero sequence number, I think it should work but largest seqno suffices for now and is easier to reason about.
We may add an API like MoveColumnFamily() that is called on the source CF side so that we can sanity check FileMetaData::fd::largest_seqno.
There was a problem hiding this comment.
This MoveColumnFamily API is used to merge (ingest) one column family's data into another column family in the same DB, right? I wonder how snapshots held by other column families could affect the data preprocessing of the ingestion preparation column family. Since it needs to do a full compaction to get rid of duplicate user keys, does that mean all snapshots should be released before this full compaction. And maybe that's requirement is not straightforward to guarantee when there are other column families.
There was a problem hiding this comment.
Yeah, snapshot can prevent such keys to be dropped. Maybe the temporary CF should in a temporary DB..
| // - ignore cf_id mismatch between cf_id in the files and the CF they are | ||
| // being ingested into. | ||
| // Warning: This ONLY works for SST files where all keys have sequence number | ||
| // zero and with no duplicated user keys (this should be guaranteed if the |
There was a problem hiding this comment.
Yes, I'm considering adding largest seqno to table property. For no duplicate user key but with non-zero sequence number, I think it should work but largest seqno suffices for now and is easier to reason about.
We may add an API like MoveColumnFamily() that is called on the source CF side so that we can sanity check FileMetaData::fd::largest_seqno.
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
Since the feature if not forward-compatible, marked the option as experimental for now in case we want a more explicit opt-in, e.g., introduce MANIFEST versioning and require setting a new MANIFEST version. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Sorry I'm confused. Won't the sequence number be zero regardless of whether we get it from the internal key footer, or from interpreting the largest_seqno as a global sequence number? Is this field meant to prepare for some future scenario where the internal key footer might not have the right sequence number? |
Sorry, by largest largest_seqno, I meant the field FileDescriptor::largest_seqno that is persisted in MANIFEST. This is the global sequence number that's assigned when the file is ingested: rocksdb/db/external_sst_file_ingestion_job.cc Line 471 in de6d0e5 |
Got it, thanks. I thought we used to have some trick, like setting |
No, wait, that's worse for downgrade compatibility, as it'd silently not interpret the Something better for downgrade compatibility could be restricting this feature to ingesting with seqno zero. |
That makes sense. I guess then we won't need the |
BTW, Zippy has some plans to use bulk loading to ingest files from another DB into a column family to restore that column family. https://docs.google.com/document/d/1T7RyAD31zaDP-cPXqW7jKpThjyOI8JtbO05CDCNafxM/edit#heading=h.mygkiilv6ocy One way that I can think of for them to do this is to do multiple bulk loading, one for each of the level of the original column family. For this to work, I think we need to support arbitrary sequence numbers from live DB's files. |
Does CreateColumnFamilyWithImport() work for that use case? |
This seems like the closest thing that's already available, for their in place back up use case. I think they just need to do something like create a new column family from importing the backed up files to restore and then drop the old column family altogether. I will check it more and discuss with them. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
2 similar comments
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cbi42 has updated the pull request. You must reimport the pull request before landing. |
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Hi @cbi42, This feature looks quite interesting and promising in terms of performance optimization. I hope you don't mind if I have a couple of questions: Is this feature already fully implemented in the pull request or is it just a prototype? It seems to "allow" users to ingest live SST files, but when I attempted to do so, the keys still could not be read in the new instance/column family, because a normal live SST usually has key sequence numbers larger than 0. Additionally, I tried to implement this on my own but still encountered issues reading the ingested keys, even after unifying the SST file version and key sequence number. Could there be any restrictions on the reader side, such as a difference in session ID, that might be preventing the keys from being read? Thank you for your time and assistance! |
|
Hi @wwl2755.
This is implemented. It's in "EXPERIMENTAL" state, but should be usable.
This is not expected. The unit tests have several example usages. Did the ingestion return OK status?
I don't think there are such restrictions. The main challenge in read path is to use a global sequence number for reading these ingested files, even though keys in such files do not have this sequence number. Requiring all keys to have sequence number = 0 makes the read path (like seeking into data blocks) easier. Also we have this assumption where all keys have unique user_key@seqno. So ingested files should not have duplicated keys since they will be assigned the same sequence number. |
Summary: ... to enable use cases like using RocksDB to merge sort data for ingestion. A new file ingestion option
IngestExternalFileOptions::allow_db_generated_filesis introduced to allows users to ingest SST files generated by live DBs instead of SstFileWriter. For now this only works if the SST files being ingested have zero as their largest sequence number AND do not overlap with any data in the DB (so we can assign seqno 0 which matches the seqno of all ingested keys).The feature is marked the option as experimental for now.
Main changes needed to enable this:
Rest of the change is mostly in new unit tests.
A previous attempt is in #5602.
Test plan: