vttablet: action lock needed to update mysql port #6432
Merged
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
enisoc
reviewed
Jul 10, 2020
| @@ -582,6 +582,12 @@ func (tm *TabletManager) findMysqlPort(retryInterval time.Duration) { | |||
| if err != nil { | |||
| continue | |||
| } | |||
| // We need to get the action lock to make sure no one | |||
There was a problem hiding this comment.
I guess pubMu is only for preventing data races when updating the local tablet, but this is a higher-level semantic race?
There was a problem hiding this comment.
Because Restore caches the tablet at the beginning and reuses it at the end. In my initial fix, I changed Restore to re-read the tablet before publishing it. But I remembered that this pattern exists everywhere else, which essentially requires the action lock to be the mutex for the tablet.
I think the more correct fix will be to change all call sites to not cache the tablet, but instead directly change it. This is the more subtle fix I had in mind.
The restore function races with the mysql port finder, and sometimes ends up overwriting the port with its own empty cached value. This "big hammer" fix makes the port finder wait till it gets the action lock before updating the mysql port. In a later cleanup, we can use a more subtle method. Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
sougou
force-pushed
the
ss-fix-mysql-port
branch
from
37b9730 to
c121f6b
Compare
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.
The restore function races with the mysql port finder, and sometimes
ends up overwriting the port with its own empty cached value.
This "big hammer" fix makes the port finder wait till it gets the
action lock before updating the mysql port. In a later cleanup,
we can use a more subtle method.
Signed-off-by: Sugu Sougoumarane ssougou@gmail.com