Temporarily disable file ingestion and LockWAL combination #12642
Conversation
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Is this why sequence number can be advanced under LockWAL()? |
Yeah, that's my guess, we are discussing more context on the task T187711884. |
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
jowlyzhang
force-pushed
the
disable_file_ingestion_for_lockwal
branch
from
aae90f1
to
c0a5489
Compare
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Btw, the documented function of LockWAL is just my attempt at reverse engineering, because the original author did not document its purpose or function. It's possible that certain logical state changes should be allowed. 🤷♂️ |
Right, I can sync with MyRocks to see if this is a legitimate combination and decide if we should test |
tools/db_crashtest.py
Outdated
| @@ -858,6 +858,9 @@ def finalize_and_sanitize(src_params): | |||
| elif (dest_params.get("use_put_entity_one_in") > 1 and | |||
| dest_params.get("use_timed_put_one_in") == 1): | |||
| dest_params["use_timed_put_one_in"] = 3 | |||
| # TODO: renable this when file ingestion is failed during wal lock. | |||
| if dest_params.get("lock_wal_one_in") != 0: | |||
There was a problem hiding this comment.
In most configurations, lock_wal_one_in is always != 0. I'm not sure there are any configurations remaining with a chance of testing ingestion
There was a problem hiding this comment.
That's a good point! I updated this to randomly disable one of the two features so that they do not run together, but can both get tested.
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
A nice solution, thanks. I'll stamp the internal diff on re-import.
Unit test update I was working on is in #12652. On pause while we're making sure we understand the requirements
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
jowlyzhang
changed the title
|
@jowlyzhang merged this pull request in 1567d50. |
Summary: We recently noticed that some memtable flushed and file ingestions could proceed during LockWAL, in violation of its stated contract. (Note: we aren't 100% sure its actually needed by MySQL, but we want it to be in a clean state nonetheless.) Despite earlier skepticism that this could be done safely (facebook#12666), I found a place to wait to wait for LockWAL to be cleared before allowing these operations to proceed: WaitForPendingWrites() Test Plan: Added to unit tests. Extended how db_stress validates LockWAL and re-enabled combination of ingestion and LockWAL in crash test, in follow-up to facebook#12642 Ran blackbox_crash_test for a long while with relevant features amplified. Suggested follow-up: fix FaultInjectionTestFS to report file sizes consistent with what the user has requested to be flushed.
Summary: We recently noticed that some memtable flushed and file ingestions could proceed during LockWAL, in violation of its stated contract. (Note: we aren't 100% sure its actually needed by MySQL, but we want it to be in a clean state nonetheless.) Despite earlier skepticism that this could be done safely (#12666), I found a place to wait to wait for LockWAL to be cleared before allowing these operations to proceed: WaitForPendingWrites() Pull Request resolved: #12652 Test Plan: Added to unit tests. Extended how db_stress validates LockWAL and re-enabled combination of ingestion and LockWAL in crash test, in follow-up to #12642 Ran blackbox_crash_test for a long while with relevant features amplified. Suggested follow-up: fix FaultInjectionTestFS to report file sizes consistent with what the user has requested to be flushed. Reviewed By: jowlyzhang Differential Revision: D57622142 Pulled By: pdillinger fbshipit-source-id: aef265fce69465618974b4ec47f4636257c676ce
…12652) Summary: We recently noticed that some memtable flushed and file ingestions could proceed during LockWAL, in violation of its stated contract. (Note: we aren't 100% sure its actually needed by MySQL, but we want it to be in a clean state nonetheless.) Despite earlier skepticism that this could be done safely (facebook#12666), I found a place to wait to wait for LockWAL to be cleared before allowing these operations to proceed: WaitForPendingWrites() Pull Request resolved: facebook#12652 Test Plan: Added to unit tests. Extended how db_stress validates LockWAL and re-enabled combination of ingestion and LockWAL in crash test, in follow-up to facebook#12642 Ran blackbox_crash_test for a long while with relevant features amplified. Suggested follow-up: fix FaultInjectionTestFS to report file sizes consistent with what the user has requested to be flushed. Reviewed By: jowlyzhang Differential Revision: D57622142 Pulled By: pdillinger fbshipit-source-id: aef265fce69465618974b4ec47f4636257c676ce
As titled. A proper fix should probably be failing file ingestion if the DB is in a lock wal state as it promises to "Freezes the logical state of the DB".