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

Fix handling race conditions on statistics writes when DDSE is MyRocks #1363

Conversation

laurynas-biveinis
Copy link
Contributor

If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

If two threads attempt to update statistics for the same object in parallel, one
of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY,
but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus
handle accordingly. The server layer patch could be avoided by making MyRocks
return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to
do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when
DDSE == MyRocks
@facebook-github-bot
Copy link

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

@laurynas-biveinis laurynas-biveinis marked this pull request as draft September 25, 2023 16:09
@laurynas-biveinis
Copy link
Contributor Author

#1363
@luqun, @hermanlee, we discussed offline that enabling RocksDB deadlock detection would allow not patching the server layer. But after investigating, it does not look that deadlock detection is related. The problem is thread 1 inserting a row to a stat table and thread 2 preparing to insert or update a row with the same primary key (GetForUpdate). There is no deadlock because thread 1 is not waiting for any locks held by thread 2, and InnoDB is returning ER_DUP_ENTRY and not ER_LOCK_DEADLOCK in this case.

If deadlock detection is excluded, then in order not to patch the server layer MyRocks would need to return ER_DUP_ENTRY. Currently it does so only for bulk loads. The comments for GetForUpdate do not suggest the returned errors are granular enough to tell the ER_DUP_ENTRY case apart from the other ones.

@laurynas-biveinis laurynas-biveinis marked this pull request as ready for review October 2, 2023 10:27
@luqun
Copy link
Contributor

luqun commented Oct 4, 2023

pushed..

@luqun luqun closed this Oct 4, 2023
facebook-github-bot pushed a commit that referenced this pull request Oct 4, 2023
#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: #1363

Differential Revision: D49595129

fbshipit-source-id: 8e304def5aa00062da3d0c8948fdfceeaf1db81e
@laurynas-biveinis laurynas-biveinis deleted the myr-ddse-dup-stats branch October 5, 2023 07:42
hermanlee pushed a commit to hermanlee/mysql-5.6 that referenced this pull request Oct 18, 2023
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
laurynas-biveinis added a commit to laurynas-biveinis/mysql-5.6 that referenced this pull request Nov 7, 2023
The fix was PR'ed in facebook#1363 without a
testcase.

Squash with f176e2b
laurynas-biveinis added a commit to laurynas-biveinis/mysql-5.6 that referenced this pull request Nov 9, 2023
The fix was PR'ed in facebook#1363 without a
testcase.

Squash with f176e2b
facebook-github-bot pushed a commit that referenced this pull request Nov 11, 2023
Summary:
The fix was PR'ed in #1363 without a testcase.

Squash with f176e2b

Pull Request resolved: #1387

Differential Revision: D51162702

fbshipit-source-id: 10b95e9108af61cf2fa053c95668e646b73dbd24
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 9, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 10, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 10, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 13, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 15, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 16, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 17, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 17, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 21, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 21, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 30, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 18, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 18, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 18, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 19, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 19, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 30, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 31, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Aug 2, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Aug 6, 2024
facebook#1363)

Summary:
If two threads attempt to update statistics for the same object in parallel, one of them may fail, and the failure is discarded. InnoDB fails with ER_DUP_ENTRY, but MyRocks may return ER_LOCK_WAIT_TIMEOUT instead in the same situation, thus handle accordingly. The server layer patch could be avoided by making MyRocks return ER_DUP_ENTRY too in the same situation, but I don't see an obvious way to do it.

This fixes main.information_schema_statistics and main.alter_table_myisam when DDSE == MyRocks

Pull Request resolved: facebook#1363

Differential Revision: D49595129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants