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

New 'RENAME TABLE ... NO LOCK CHECK' optional clause. #426

Closed

Conversation

shlomi-noach
Copy link

@shlomi-noach shlomi-noach commented Oct 23, 2022

This PR proposes an optional NO LOCK CHECK clause to the RENAME TABLE statement, like so:

rename table t1 to t0, t2 to t1, t0 to t2 no lock check;

This clause disables (skips) the existing pre-RENAME validation that all relevant tables are either all locked or none is locked.

A bit of context. From the docs:

Prior to MySQL 8.0.13, to execute RENAME TABLE, there must be no tables locked with LOCK TABLES.

So pre 8.0.13 any attempt to LOCK TABLES any_table_whatsoever WRITE; RENAME TABLE ... would result with error.

In 8.0.13 a check is added: either no table is locked, or all affected tables must be locked. This was developed to assist 3rd party online schema change tools, specifically gh-ost (much appreciated!), to manage the final cut-over phase. However, as it turned out, this implementation created a different kind of complexity to gh-ost (for reasons we can elaborate on if so desired). Today, as I'm looking into both gh-ost and vitess online schema changes, I'm again facing the cut-over issue.

For even more context, the following should be considered:

  • The fact the tables are not locked up-front is not really an issue. As evidence, pre 8.0.13 locks were impossible.
  • The RENAME operation grabs all necessary locks, whether locks were present beforehand or not.
  • The current code comments mention:
    In theory, we could disregard whether they locked or not and just try
    to acquire exclusive metadata locks on them, but this is too deadlock
    prone.

    However, I think the risk of deadlock is very low, and easily mitigated by SET LOCK_WAIT_TIMEOUT to any reasonable value, which then works well to resolve a table-lock deadlock scenario.
  • The ideal logic for all asynchronous online-schema-change tools, fb-osc, gh-ost and vitess is to LOCK WRITE the original table only, then keep digesting what remains of the changelog, applying it to the "shadow table", then flipping both.

With this new optional clause, an OSC like gh-ost or vitess would therefore run:

  1. Connection #1: LOCK TABLES original_table WRITE
  2. Connection #2: apply any remaining INSERT/DELETE/UPDATE on the shadow table
  3. Connection #1: RENAME TABLE original_table TO _old, _shadow_table TO original_table or RENAME TABLE original_table TO _swap, _shadow_table TO original_table, _swap TO _shadow_table
    The RENAME operation escalates connection #1 locks to hold both original_table and _shadow_table
  4. Connection #1: UNLOCK TABLES
  5. Done

That's a much shorter, less error prone, and overall less locking, flow than the one described in http://code.openark.org/blog/mysql/solving-the-non-atomic-table-swap-take-iii-making-it-atomic (see additional writeup here: vitessio/vitess#11460 (comment)). But the flow is currently impossible because it grabs a lock on a single table out of the two/more tables used in the RENAME statement, which fails the RENAME.

And so this contribution offers:

  • An optional clause NO LOCK CHECK
  • Which changes locking check behavior of a RENAME statement
  • Backwards compatible if clause is not provided
  • To be used, I guess, exclusively by OSC tools
  • Will benefit gh-ost, vitess and fb-osc

Thank you for your consideration!

…a 'RENAME TABLE' statement does not check for existing table write locks before operating (it does keep escalating locks as needed upon operation, as usual)

Signed-off-by: Shlomi Noach <[email protected]>
@mysql-oca-bot
Copy link

Hi, thank you for submitting this pull request. In order to consider your code we need you to sign the Oracle Contribution Agreement (OCA). Please review the details and follow the instructions at https://oca.opensource.oracle.com/
Please make sure to include your MySQL bug system user (email) in the returned form.
Thanks

@shlomi-noach
Copy link
Author

I have signed the OCA a few years ago and have made contributions before. Is there a need to renew/re-sign the OCA?

@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment:
"I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it."
Thanks

@mysql-admin
Copy link

mysql-admin commented Oct 24, 2022

Hi Shlomi,
No need to re-sign - for some reason your OCA record got garbled - fixed

Please confirm contribution is under the OCA as posted by the bot above

Thanks
==Omer

@shlomi-noach
Copy link
Author

I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow
bug http://bugs.mysql.com/bug.php?id=108864 for updates.
Thanks

@shlomi-noach
Copy link
Author

I am actually keen to make a change here: instead of modifying the SQL syntax (thus, yacc, lex and all the call stack), I would like to introduce a global/session variable such as rename_tables_skip_lock_check (defaults false for standard behavior). WDYT?

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

Successfully merging this pull request may close these issues.

3 participants