MDEV-24948: thd_need_wait_reports() hurts performance #3823
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reduce the cost of thd_rpl_deadlock_check() which is used to do parallel replication conflict detecton. 1. Cache the value of thd_need_wait_reports() in InnoDB trx_t, and only call thd_rpl_deadlock_check() if required. 2. Remove the FL_WAITED flag in GTID. This flag was set if a transaction did any lock wait inside InnoDB on the master. In optimistic parallel replication on the slave, such transaction would not run in parallel with any prior transactions. It is not clear what the value of this mechanism is. If a transaction did not wait on the master, it can still conflict on the slave. And even if it did have to wait on the master, that does not mean that it will need to wait on the slave, or that such wait will cause a conflict or other problem. So waiting for all prior transactions on the slave in FL_WAITED transactions might slow down things rather than speed up. 3. Only request thd_rpl_deadlock_check() calls for non-replication THDs if the --binlog-commit-wait-count option is enabled. Such wait might have to timeout, if the waiting commits are blocking other transactions from joining the group commit, so the wait reports are used to terminate the wait early. After this patch, setting --binlog-commit-wait-count non-zero will only have effect for newly created connections. With these changes, unless the non-default --binlog-commit-wait-count option is used, the overhead of thd_rpl_deadlock_check() is reduced to a single conditional in non-parallel-replication threads. Signed-off-by: Kristian Nielsen <[email protected]>
|
|
dr-m
reviewed
Feb 12, 2025
Comment on lines
2090
to
+2091
| const bool rpl= trx->mysql_thd && innodb_deadlock_detect && | ||
| thd_need_wait_reports(trx->mysql_thd); | ||
| trx->need_wait_report; |
There was a problem hiding this comment.
As far as I can tell, we could remove the trx->mysql_thd && part. I’d check innodb_deadock_detect last.
Comment on lines
386
to
+388
| assert_freed(); | ||
| trx_sys.rw_trx_hash.put_pins(this); | ||
| need_wait_report= false; |
There was a problem hiding this comment.
It would be cleaner to clear the need_wait_report in trx_t::commit_cleanup() or similar, and extend assert_freed() with ut_ad(!need_wait_report).
| @@ -544,6 +545,7 @@ void trx_disconnect_prepared(trx_t *trx) | |||
| trx->read_view.close(); | |||
| trx_sys.trx_list.freeze(); | |||
| trx->is_recovered= true; | |||
| trx->need_wait_report= false; | |||
There was a problem hiding this comment.
Zero initialization should be guaranteed by a debug assertion that you should add to trx_create().
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reduce the cost of thd_rpl_deadlock_check() which is used to do parallel
replication conflict detecton.
Cache the value of thd_need_wait_reports() in InnoDB trx_t, and only call
thd_rpl_deadlock_check() if required.
Remove the FL_WAITED flag in GTID. This flag was set if a transaction did
any lock wait inside InnoDB on the master. In optimistic parallel
replication on the slave, such transaction would not run in parallel with
any prior transactions. It is not clear what the value of this mechanism is.
If a transaction did not wait on the master, it can still conflict on the
slave. And even if it did have to wait on the master, that does not mean
that it will need to wait on the slave, or that such wait will cause a
conflict or other problem. So waiting for all prior transactions on the
slave in FL_WAITED transactions might slow down things rather than speed up.
Only request thd_rpl_deadlock_check() calls for non-replication THDs if
the --binlog-commit-wait-count option is enabled. Such wait might have to
timeout, if the waiting commits are blocking other transactions from joining
the group commit, so the wait reports are used to terminate the wait early.
After this patch, setting --binlog-commit-wait-count non-zero will only have
effect for newly created connections.
With these changes, unless the non-default --binlog-commit-wait-count option
is used, the overhead of thd_rpl_deadlock_check() is reduced to a single
conditional in non-parallel-replication threads.
Signed-off-by: Kristian Nielsen [email protected]