Assert unlimited max_open_files for FIFO compaction. #8172
Conversation
thejchap
changed the title
thejchap
force-pushed
the
bug/fifo-compaction-with-ttl-and-max-open-files-1
branch
from
593c979
to
6f38b78
Compare
There was a problem hiding this comment.
Looks good so far. Do you mind also mentioning this under "Bug Fixes" section in the "HISTORY.md" file?
It looks like some tests relied on the now unsupported combination of options. You can try running make check -j$(nproc) locally (if that fails and it's not obvious what caused it, try make watch-log) to find all such cases.
thejchap
force-pushed
the
bug/fifo-compaction-with-ttl-and-max-open-files-1
branch
7 times, most recently
from
756785a
to
a43af28
Compare
utilities/option_change_migration/option_change_migration_test.cc
Outdated
Show resolved
Hide resolved
Resolves facebook#8014 - Add an assertion on `DB::Open` to ensure `db_options.max_open_files` is unlimited if FIFO Compaction is being used. - This is to align with what the docs mention and to prevent premature data deletion. - Update tests to work with this assertion. Test Plan: 'make check -j$(nproc)`
thejchap
force-pushed
the
bug/fifo-compaction-with-ttl-and-max-open-files-1
branch
from
a43af28
to
fe19286
Compare
thejchap
changed the title
|
@thejchap has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| if (cf_options.compaction_style == kCompactionStyleFIFO && | ||
| db_options.max_open_files != -1 && cf_options.ttl > 0) { |
There was a problem hiding this comment.
Can you explain the ttl > 0? If ttl == 0, is it okay if max_open_files != -1 ?
There was a problem hiding this comment.
As far as we know, the max_open_files == -1 is only needed to ensure the files' creation_time properties are read, which are used for deciding to trigger TTL compaction. When ttl == 0, TTL compaction is disabled.
Resolves #8014
DB::Opento ensuredb_options.max_open_filesis unlimited if FIFO Compaction is being used.Test Plan:
$ make check -j$(nproc) Generated TARGETS Summary: - 6 libs - 0 binarys - 180 tests