Track WAL in MANIFEST: LogAndApply WAL events to MANIFEST #7601
Conversation
ghost
requested a review
from 
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Thanks for the PR @Cheng-Chang ! Please see some questions/comments below.
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
|
@ltamasi Thanks for the review! PTAL. |
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Existing API `VerifyChecksum()` allows application to verify sst files' block checksums. Since whole file, user-specified checksum is tracked in MANIFEST, we can expose a new API to verify sst files' file checksums. ``` // Compute table file checksums if applicable and compare with MANIFEST. // Returns OK if no file has mismatching whole-file checksum. Status DB::VerifyFileChecksums(const ReadOptions& /*read_options*/); ``` Pull Request resolved: #7578 Test Plan: make check Reviewed By: pdillinger Differential Revision: D24436783 Pulled By: riversand963 fbshipit-source-id: 52b51519b842f2b3c4e3351998a97c86cbec85b3
Summary: In `BuildTable()`, we call `builder->Finish()` before evaluating `builder->NeedCompact()`. However, we call `builder->NeedCompact()` before `builder->Finish()` in compaction job. This can be wrong because the table properties collectors may rely on the success of `Finish()` to provide correct result for `NeedCompact()`. Test plan (on devserver): make check Pull Request resolved: #7627 Reviewed By: ajkr Differential Revision: D24728741 Pulled By: riversand963 fbshipit-source-id: 5a0dce244e14eb1106c4f87021e6bebca82b486e
Summary: `llvm-mirror/clang` is archived. Get the `clang-format-diff.py` file from the active source. Pull Request resolved: #7609 Reviewed By: ajkr Differential Revision: D24711608 Pulled By: pdillinger fbshipit-source-id: b115d8765ff23fbb8190290a170de21565daba84
Summary: The original test nests a lot of `try` blocks. This PR flattens these blocks into independent blocks, so that each `try` block closes the DB before opening the next DB instance. Pull Request resolved: #7608 Test Plan: watch the existing java tests to pass Reviewed By: zhichao-cao Differential Revision: D24611621 Pulled By: cheng-chang fbshipit-source-id: d486c5d37ac25d4b860d739ef2cdd58e6064d42d
Summary: This test often times out in internal test infra. Pull Request resolved: #7637 Test Plan: watch test to pass Reviewed By: ajkr Differential Revision: D24763939 Pulled By: cheng-chang fbshipit-source-id: 6564ee2ef637e9faf6688d4b6a5d74a72a51c5e8
Summary: Tries to fix by skipping fsync. Pull Request resolved: #7638 Test Plan: watch the tests to pass Reviewed By: jay-zhuang Differential Revision: D24764355 Pulled By: cheng-chang fbshipit-source-id: 9c21b177709025ca1943066d94da89324ed47655
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@cheng-chang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@Cheng-Chang merged this pull request in 1e40696. |
) Summary: When a WAL is synced, an edit is written to MANIFEST. After flushing memtables, the obsoleted WALs are piggybacked to MANIFEST while writing the new L0 files to MANIFEST. Pull Request resolved: facebook#7601 Test Plan: `track_and_verify_wals_in_manifest` is enabled by default for all tests extending `DBBasicTest`, and in db_stress_test. Unit test `wal_edit_test`, `version_edit_test`, and `version_set_test` are also updated. Watch all tests to pass. Reviewed By: ltamasi Differential Revision: D24553957 Pulled By: cheng-chang fbshipit-source-id: 66a569ff1bdced38e22900bd240b73113906e040
When a WAL is synced, an edit is written to MANIFEST.
After flushing memtables, the obsoleted WALs are piggybacked to MANIFEST while writing the new L0 files to MANIFEST.
Test Plan:
track_and_verify_wals_in_manifestis enabled by default for all tests extendingDBBasicTest, and in db_stress_test.Unit test
wal_edit_test,version_edit_test, andversion_set_testare also updated.Watch all tests to pass.