Skip to content

Before the successful renaming, a session accessed the ghost table, w… #1536

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

abaowhy
Copy link

@abaowhy abaowhy commented Apr 21, 2025

…hich had already unlocked the original table.

Before the successful renaming, a session accessed the ghost table, which had already unlocked the original table.
There is a very small probability that other sessions dml operations on the original table will occur,
and this dml operation will appear in the original table after renaming, resulting in data loss.
https://cloud.tencent.com/developer/article/2303777

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.

Related issue: https://github.com/github/gh-ost/issues/0123456789

Further notes in https://github.com/github/gh-ost/blob/master/.github/CONTRIBUTING.md
Thank you! We are open to PRs, but please understand if for technical reasons we are unable to accept each and any PR

Description

This PR [briefly explain what it does]
Only when the rename session is waiting for the lock of the original table in performance_schema.metadata_locks, will the lock of the original table be released, and finally rename can be executed. If rename does not wait for the lock, it will exit gh-ost.

In case this PR introduced Go code changes:

…hich had already unlocked the original table.

There is a very small probability that other sessions dml operations on the original table will occur,
and this dml operation will appear in the original table after renaming, resulting in data loss.
@arthurschreiber
Copy link
Member

@abaowhy Hey, thanks for the bug report and the code changes! 🙇‍♂️ ❤️

Is there a straightforward way to reproduce this issue? It'd be good to have a test cases that could show that a) this issue exists and b) the proposed code changes fix that issue. 🤔

@abaowhy
Copy link
Author

abaowhy commented Apr 24, 2025

@abaowhy Hey, thanks for the bug report and the code changes! 🙇‍♂️ ❤️

Is there a straightforward way to reproduce this issue? It'd be good to have a test cases that could show that a) this issue exists and b) the proposed code changes fix that issue. 🤔

Cutover single testing is quite difficult, so I first provided a picture to illustrate
image

@cenkore
Copy link

cenkore commented Apr 29, 2025

It looks similar to this PR #1269 .

@abaowhy
Copy link
Author

abaowhy commented Apr 30, 2025

@abaowhy Hey, thanks for the bug report and the code changes! 🙇‍♂️ ❤️

Is there a straightforward way to reproduce this issue? It'd be good to have a test cases that could show that a) this issue exists and b) the proposed code changes fix that issue. 🤔

@arthurschreiber I have provided test cases that issue can be stably reproduced on master and fixed in PR.

@abaowhy
Copy link
Author

abaowhy commented May 8, 2025

@meiji163 I have provided test cases, please take some time to review, and if there are any issues, I will promptly correct them.

@abaowhy
Copy link
Author

abaowhy commented May 12, 2025

@meiji163 I have fixed three issues during the inspection. Could you please take some time to check again

@abaowhy
Copy link
Author

abaowhy commented May 16, 2025

@meiji163 @timvaillancourt @rashiq I have provided test cases that issue can be stably reproduced on master and fixed in PR,please take some time to review, and if there are any issues, I will promptly correct them.

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.

4 participants