Support options.ttl with options.max_open_files = -1 #6060
Conversation
Summary: Previously, options.ttl cannot be set with options.max_open_files = -1, because it makes use of creation_time field in table properties, which is not available unless max_open_files = -1. With this commit, the information will be stored in manifest and when it is available, will be used instead. Note that, this change will break forward compatibility for release 5.1 and older. Test Plan: Extend existing test case to options.max_open_files != -1, and simulate backward compatility in one test case by forcing the value to be 0.
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@siying has updated the pull request. Re-import the pull request |
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| // in turn be outputs for compact older SST files. We track the memtable | ||
| // flush timestamp for the oldest SST file that eventaully contribute data | ||
| // to this file. 0 means the information is not available. | ||
| uint64_t oldest_ancester_time = 0; |
There was a problem hiding this comment.
nit: Maybe initialize to kUnknownOldestAncesterTime instead of 0 here.
There was a problem hiding this comment.
I issued landing but just realized that I forgot to apply this comment. I'll try to do it as a follow up.
HISTORY.md
Outdated
| @@ -4,6 +4,7 @@ | |||
| * Fix data corruption casued by output of intra-L0 compaction on ingested file not being placed in correct order in L0. | |||
|
|
|||
| ### Public API Change | |||
| * Database generated will not be able to be opened by version 5.1 or older, even if no new feature is used. | |||
There was a problem hiding this comment.
Is 5.1 the version where we introduced kNewFile4 manifest format?
There was a problem hiding this comment.
I thought it was 5.2, so I mentioned it here. But I rechecked and it seems to be introduced in 4.2: b77eb16
Then maybe I don't need to have this line. Let me double check.
There was a problem hiding this comment.
Yes I validated that release 4.2 can open the DB created by DB with this change. I'll remove this line.
|
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@siying is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request has been merged in d8c28e6. |
Summary: facebook#6060 broke forward compatiblity for releases from 3.10 to 4.2. Update HISTORY.md to mention it. Also remove it from the compatibility tests.
* upstream/master: (572 commits) Work around weird unused errors with Mingw (facebook#6075) Support options.max_open_files = -1 with periodic_compaction_seconds (facebook#6090) Fix HISTORY.md for 6.6.0 (facebook#6096) Expose and elaborate FilterBuildingContext (facebook#6088) Fix compilation under MSVC VS2015 (facebook#6081) Add shared library for musl-libc (facebook#3143) Refactor and clean up the code that reads a blob from a file (facebook#6093) Allow fractional bits/key in BloomFilterPolicy (facebook#6092) Refactor blob file creation logic (facebook#6066) Use lowercase for shlwapi.lib rpcrt4.lib (facebook#6076) Fix naming of library on PPC64LE (facebook#6080) Small improvements to Docker build for RocksJava (facebook#6079) Remove unused/undefined ImmutableCFOptions() (facebook#6086) Update 3rd-party libraries used by RocksJava (facebook#6084) Make default value of options.ttl to be 30 days when it is supported. (facebook#6073) Ignore value of BackupableDBOptions::max_valid_backups_to_open when B… (facebook#6072) Update HISTORY.md for forward compatibility (facebook#6085) Support ttl in Universal Compaction (facebook#6071) Fix the constness issues around autovector::iterator_impl's dereference operators (facebook#6057) Support options.ttl with options.max_open_files = -1 (facebook#6060) ...
Summary: Previously, options.ttl cannot be set with options.max_open_files = -1, because it makes use of creation_time field in table properties, which is not available unless max_open_files = -1. With this commit, the information will be stored in manifest and when it is available, will be used instead. Note that, this change will break forward compatibility for release 5.1 and older. Pull Request resolved: facebook#6060 Test Plan: Extend existing test case to options.max_open_files != -1, and simulate backward compatility in one test case by forcing the value to be 0. Differential Revision: D18631623 fbshipit-source-id: 30c232a8672de5432ce9608bb2488ecc19138830
Summary: facebook#6060 broke forward compatiblity for releases from 3.10 to 4.2. Update HISTORY.md to mention it. Also remove it from the compatibility tests. Pull Request resolved: facebook#6085 Differential Revision: D18691694 fbshipit-source-id: 4ef903783dc722b8a4d3e8229abbf0f021a114c9
Summary:
Previously, options.ttl cannot be set with options.max_open_files = -1, because it makes use of creation_time field in table properties, which is not available unless max_open_files = -1. With this commit, the information will be stored in manifest and when it is available, will be used instead.
Note that, this change will break forward compatibility for release 5.1 and older.
Test Plan: Extend existing test case to options.max_open_files != -1, and simulate backward compatility in one test case by forcing the value to be 0.