]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/commit
Fix sequence number bump logic in multi-CF SST ingestion (#9005)
authorAndrew Kryczka <andrewkr@fb.com>
Thu, 14 Oct 2021 17:45:19 +0000 (10:45 -0700)
committerAndrew Kryczka <andrewkr@fb.com>
Thu, 14 Oct 2021 17:45:19 +0000 (10:45 -0700)
commitfa4e0558bf91f581c608bcda57f52d194f9d8940
tree8ca2e873b71b03d68bcd9987ebdec5304da8317d
parent3ebe8658d06dbb3958036887d8601a5290c04ba1
Fix sequence number bump logic in multi-CF SST ingestion (#9005)

Summary:
The code in `IngestExternalFiles()` that bumps the DB's sequence number
depending on what seqnos were assigned to the files has 3 bugs:

1) There is an assertion that the sequence number is increased in all the
affected column families, but this is unnecessary, it is fine if some files can
stick to a lower sequence number. It is very easy to hit the assertion: it is
sufficient to insert 2 files in 2 CFs, one which overlaps the CF and one that
doesn't (for example the CF is empty). The line added in the
`IngestFilesIntoMultipleColumnFamilies_Success` test makes the assertion fail.

2) SetLastSequence() is called with the sum of all the bumps across CFs, but we
should take the maximum instead, as all CFs start with the current seqno and bump
it independently.

3) The code above is accidentally under a `#ifndef NDEBUG`, so it doesn't run in
optimized builds, so some files may be assigned seqnos from the future.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9005

Test Plan:
Added line in `IngestFilesIntoMultipleColumnFamilies_Success` that
triggers the assertion, verified that the test (and all the others) pass after
the fix.

Reviewed By: ajkr

Differential Revision: D31597892

Pulled By: ot

fbshipit-source-id: c2d3237f90290df1178736ace8653a9623f5a770
HISTORY.md
db/db_impl/db_impl.cc
db/external_sst_file_ingestion_job.h
db/external_sst_file_test.cc