BlobDB: handle the case when OnCompactionCompleted precedes OnFlushCompleted

[ltamasi]

Summary:
BlobDB keeps track of the mapping between SSTs and blob files using
the OnFlushCompleted and OnCompactionCompleted callbacks of
the Listener interface: upon receiving a flush notification, a link is added
between the newly flushed SST and the corresponding blob file; for
compactions, links are removed for the inputs and added for the outputs.
In certain cases, it is possible for the compaction notification that results
in a link being removed to precede the flush notification that establishes
the link (see #6338).
The earlier code did not handle this case, resulting in assertion failures.
With the patch, BlobDB stores unmatched link removals in a separate set,
and reconciles them with link additions as they arrive.

Test Plan:
Added a unit test that simulates this case.

[facebook-github-bot]

@ltamasi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[burtonli]

I think handling OnCompactionCompleted precedes OnFlushCompleted inside blob db_impl may create issue for MarkUnreferencedBlobFilesObsolete(). If OnCompactionCompleted arrives first and the blob file happens to be the oldest file (e.g.: the sst file is the only file in the db, and it is triggered compaction by reaching the level base size), then the blob file may be marked as obsolete in BlobDBImpl::ProcessCompactionJobInfo() before OnFlushCompleted call.

I have a proposal to fix the issue by always make sure OnFlushCompleted is always called ahead of OnCompactionBegin and OnCompactionCompleted for the same file, which seems to be a more nature way for other external consumers to built their similar logic like BlobDB. Think about how does an external service to handle a file starts compaction before flushed.
https://github.com/facebook/rocksdb/pull/6342

[ltamasi]

Considering that this is not a threading issue as originally thought (since the notification for the trivial move compaction deterministically precedes the notification for the flush), it would be better to fix this in RocksDB itself. Closing in favor of #6342 .

[ltamasi]

I think handling OnCompactionCompleted precedes OnFlushCompleted inside blob db_impl may create issue for MarkUnreferencedBlobFilesObsolete(). If OnCompactionCompleted arrives first and the blob file happens to be the oldest file (e.g.: the sst file is the only file in the db, and it is triggered compaction by reaching the level base size), then the blob file may be marked as obsolete in BlobDBImpl::ProcessCompactionJobInfo() before OnFlushCompleted call.

We actually have protection in place against that but I do agree that this should ideally be fixed in the RocksDB core.