Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

concurrent flush cause data loss after GC #93

Closed
yiwu-arbug opened this issue Oct 8, 2019 · 0 comments
Closed

concurrent flush cause data loss after GC #93

yiwu-arbug opened this issue Oct 8, 2019 · 0 comments
Assignees

Comments

@yiwu-arbug
Copy link
Collaborator

Titan use event listener to hook OnFlushCompleted. When the event triggered, Titan assume corresponding memtable has been converted to SST, and mark blob file generated from the flush as normal file, so that GC can pick the file up. However, the assumption is not correct. If there are concurrent flush happened, OnFlushCompleted can fire before memtable being converted to SST (facebook/rocksdb#5892). If GC pick up the blob file before memtable flush finished, it can receive is_blob_index=false for all keys in the blob file (since they are still in memtable), and drop all data in the file by mistake, causing data loss.

Reproduced with unit test: yiwu-arbug@5172201

@yiwu-arbug yiwu-arbug self-assigned this Oct 8, 2019
@yiwu-arbug yiwu-arbug added the bug label Oct 14, 2019
yiwu-arbug pushed a commit to tikv/rocksdb that referenced this issue Oct 16, 2019
Move `WaitForFlushMemTable` API to public. It is needed for hotfix of tikv/titan#93

Signed-off-by: Yi Wu <yiwu@pingcap.com>
yiwu-arbug pushed a commit to tikv/rocksdb that referenced this issue Oct 16, 2019
Move `WaitForFlushMemTable` API to public. It is needed for hotfix of tikv/titan#93

Signed-off-by: Yi Wu <yiwu@pingcap.com>
yiwu-arbug pushed a commit that referenced this issue Oct 16, 2019
Summary:
Temporary fix for #93. Fixing it by waiting until no flush is ongoing before starting a GC job. The drawback is GC can starve if flush runs so frequent that there's no gap between flush jobs.

Test Plan:
See the new test. The test will fail with "Corruption: Missing blob file" error before the fix.
yiwu-arbug pushed a commit to yiwu-arbug/titan that referenced this issue Oct 16, 2019
Summary:
Temporary fix for tikv#93. Fixing it by waiting until no flush is ongoing before starting a GC job. The drawback is GC can starve if flush runs so frequent that there's no gap between flush jobs.

Test Plan:
See the new test. The test will fail with "Corruption: Missing blob file" error before the fix.
yiwu-arbug pushed a commit that referenced this issue Oct 16, 2019
Cherry-picking the following patches for fixing #93 
```
0a3f87a 2019-10-15 yiwu@pingcap.com     Temp fix for data loss caused by concurrent flush (#96)
8ac5003 2019-10-16 zbk602423539@gmail.. merge BackgroundGC with TEST_StartGC (#94)
b9915d9 2019-10-07 zbk602423539@gmail.. check nullptr (#91)
```
@yiwu-arbug yiwu-arbug removed the bug label Dec 21, 2019
tabokie pushed a commit to tabokie/rocksdb that referenced this issue May 11, 2022
Move `WaitForFlushMemTable` API to public. It is needed for hotfix of tikv/titan#93

Signed-off-by: Yi Wu <yiwu@pingcap.com>
Signed-off-by: tabokie <xy.tao@outlook.com>
tabokie pushed a commit to tikv/rocksdb that referenced this issue May 12, 2022
Move `WaitForFlushMemTable` API to public. It is needed for hotfix of tikv/titan#93

Signed-off-by: Yi Wu <yiwu@pingcap.com>
Signed-off-by: tabokie <xy.tao@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants