Ignore kBottommostFiles compaction logic when allow_ingest_behind #10767
Conversation
cbi42
force-pushed
the
udt-bottommost-compaction
branch
from
639383a to
2048697
Compare
|
@cbi42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cbi42
force-pushed
the
udt-bottommost-compaction
branch
from
2048697 to
3955fc2
Compare
|
@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. |
| auto snapshot = db_->GetSnapshot(); | ||
| // Bump up oldest_snapshot_seqnum_ in VersionStorageInfo. | ||
| db_->ReleaseSnapshot(snapshot); |
There was a problem hiding this comment.
Does ComputeBottommostFilesMarkedForCompaction() pick the L1 file even without snapshot?
There was a problem hiding this comment.
I think so. The criteria in ComputeBottommostFilesMarkedForCompaction is satisfied. The sequence number in the L1 file is not zeroed out due to allow_ingest_behind.
There was a problem hiding this comment.
If you mean not taking snapshot in this test, then no. Since oldest_snapshot_seqnum_ would be 0 and only files with 0 < largest_seqno < oldest_snapshot_seqnum_ is compacted.
There was a problem hiding this comment.
Maybe oldest_snapshot_seqnum_ isn't set properly when there's no snapshots. Anyways it's not very important.
| auto snapshot = db_->GetSnapshot(); | ||
| // Bump up oldest_snapshot_seqnum_ in VersionStorageInfo. | ||
| db_->ReleaseSnapshot(snapshot); |
There was a problem hiding this comment.
Maybe oldest_snapshot_seqnum_ isn't set properly when there's no snapshots. Anyways it's not very important.
cbi42
force-pushed
the
udt-bottommost-compaction
branch
from
3955fc2 to
673e61e
Compare
|
@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
force-pushed
the
udt-bottommost-compaction
branch
from
673e61e to
7555812
Compare
|
@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. |
…cebook#10767) Summary: fix for facebook#10752 where RocksDB could be in an infinite compaction loop (with compaction reason kBottommostFiles) if allow_ingest_behind is enabled and the bottommost level is unfilled. Pull Request resolved: facebook#10767 Test Plan: Added a unit test to reproduce the compaction loop. Reviewed By: ajkr Differential Revision: D40031861 Pulled By: ajkr fbshipit-source-id: 71c4b02931fbe507a847632905404c9b8fa8c96b
…0767) Summary: fix for #10752 where RocksDB could be in an infinite compaction loop (with compaction reason kBottommostFiles) if allow_ingest_behind is enabled and the bottommost level is unfilled. Pull Request resolved: #10767 Test Plan: Added a unit test to reproduce the compaction loop. Reviewed By: ajkr Differential Revision: D40031861 Pulled By: ajkr fbshipit-source-id: 71c4b02931fbe507a847632905404c9b8fa8c96b
Summary: fix for #10752 where RocksDB could be in an infinite compaction loop (with compaction reason kBottommostFiles) if allow_ingest_behind is enabled and the bottommost level is unfilled.
Test plan: Added a unit test to reproduce the compaction loop.