FlushMemTable return ok but memtable does not synchronize flush

[zhuchong0329]

Fix #8046 : FlushMemTable return ok but memtable does not synchronize flush. The way to fix it is to expose RecoveryError.

[mrambacher]

I think this change is going to trigger a lot of failures in the "ASSERT_STATUS_CHECKED" cases.

A better approach may be to leave the code around line 2127 alone and add/copy line 2130-31 to around 2165.

[zhuchong0329]

@mrambacher , Thanks for the reminder. I did ignore ROCKSDB_ASSERT_STATUS_CHECKED.

[ajkr]

@zhuchong0329 this seems like a useful fix. Do you plan to try making it work with ASSERT_STATUS_CHECKED? I think you can get rid of all the early returns (e.g., L2125) and instead make them set s to a non-OK Status. Then add s.ok() to all the conditionals/loop conditions to make sure nothing meaningful executes and nothing overwrites s for the rest of the function.

[facebook-github-bot]

@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[jay-zhuang]

LGTM.

Fixed ASSERT_STATUS_CHECKED as @ajkr suggested.

[riversand963]

What's our plan with this PR? @jay-zhuang

[facebook-github-bot]

@zhuchong0329 has updated the pull request. You must reimport the pull request before landing.

[ajkr]

LGTM too.

[facebook-github-bot]

@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.