KEMBAR78
avoid to send drop cutover sentry table to mysql twice v2 by MOON-CLJ · Pull Request #745 · github/gh-ost · GitHub
Skip to content

Conversation

@MOON-CLJ
Copy link
Contributor

@MOON-CLJ MOON-CLJ commented May 25, 2019

this pr is another way to deal with issue: #737

the first two commits are the same as #743 ,instead use get_lock,this pr use tableUnlocked channel to sync。

MOON-CLJ and others added 5 commits May 22, 2019 21:00
atomicCutOver use tableUnlocked to wait AtomicCutOverMagicLock completed,
avoid to concurrency call AtomicCutOverMagicLock come from retrying atomicCutOver
Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @MOON-CLJ and sorry for the late reply.
I think your change preserves the current behavior. However, I do not see how it prevents MTS from running the potential two drops in the same transaction again. The two drops, even as both created in the same function AtomicCutOverMagicLock(), can (and I expect that will) be applied by two different connections. Thus, replication stream can still pick those two drops in MTS.

@MOON-CLJ
Copy link
Contributor Author

@shlomi-noach yes, you're right。

can we just delete the logic "defer this.DropAtomicCutOverSentryTableIfExists()"? because a successful cutover will drop the sentry table eventually。or do you have any other idea?

ps, even this bug is even more a MTS's bug,i am thinking what can we do in gh-ost to eliminate the pain。

@shlomi-noach
Copy link
Contributor

OK, a less-pretty and yet simple solution is to protect the dropping of the table by a mutex, and always skip the 2nd operation. It is imperative that the table is dropped no matter what, which is why the defer DropAtomicCutOverSentryTableIfExists() is there.

@MOON-CLJ
Copy link
Contributor Author

MOON-CLJ commented Jun 11, 2019

@shlomi-noach "to protect the dropping of the table by a mutex", yes, can i gave another pr for this?

@shlomi-noach
Copy link
Contributor

@MOON-CLJ sure, thank you!

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.

2 participants