Bolt and importer changes #14736
Merged
Bolt and importer changes #14736
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
houndci-bot
reviewed
Mar 11, 2019
ethervoid
force-pushed
the
importer_with_bolt
branch
from
da75aa3
to
02a57b9
Compare
houndci-bot
reviewed
Mar 11, 2019
gonzaloriestra
suggested changes
Mar 11, 2019
There was a problem hiding this comment.
Are you planning to add some tests for the new behavior?
Regarding the importer, the idea is to take the lock in order to avoid GTM executions, but allow it to run if it's already locked, right? What happens if it's locked when it starts but then is released? GTM could be called again and run, couldn't it? Why not using it the same way as GTM (with retriable)?
houndci-bot
reviewed
Mar 12, 2019
javitonino
reviewed
Mar 13, 2019
javitonino
reviewed
Mar 13, 2019
javitonino
reviewed
Mar 13, 2019
houndci-bot
reviewed
Mar 13, 2019
houndci-bot
reviewed
Mar 13, 2019
houndci-bot
reviewed
Mar 13, 2019
javitonino
suggested changes
Mar 13, 2019
houndci-bot
reviewed
Mar 13, 2019
|
@javitonino 3rd round? |
javitonino
reviewed
Mar 14, 2019
houndci-bot
reviewed
Mar 14, 2019
ethervoid
force-pushed
the
importer_with_bolt
branch
from
080f246
to
cf01b59
Compare
|
@javitonino @gonzaloriestra added all the comments and now I'm going to add ghost_table_manager tests in order to test the part |
gonzaloriestra
suggested changes
Mar 14, 2019
|
Looks OK but CI does not agree |
You probably are wondering why is the purpose of this. The main reason to execute a "critical section" without acquiring the mutex (Bolt) is because we only need that the lock has been acquired so we know for sure that Ghost Tables are not going to be executed. In which cases are we going to need this? Mainly in the register part of the importer where we're going to perform DDL operations that are going to trigger ghost tables jobs and that could coincide at the same time with other DDL operations leading into a race condition.
We've added the possibility to retry critical sections of code if Bolt received an attempt to lock and that block of code is marked as retriable. Why are we doing this? Because we don't want to miss changes in the user's schema because Bolt was already acquired. For example, if some user is doing intensive DDL operations we want to sync all the changes so to achieve that we need to inform Bolt that we have other processes trying to execute Ghost tables at the same time and instead of releasing the lock, we mark it asking for retry and at the end we re-execute the same critical section again.
This happens when other process tried to acquire Bolt and was not able to do it.
ethervoid
force-pushed
the
importer_with_bolt
branch
from
e489852
to
9bd28aa
Compare
ethervoid
force-pushed
the
importer_with_bolt
branch
from
d30c559
to
42edb60
Compare
ethervoid
force-pushed
the
importer_with_bolt
branch
from
42edb60
to
a9dda22
Compare
javitonino
approved these changes
Mar 15, 2019
|
✔️ Acceptance OK Importer and GTM calls from dashboard seem to work as before. Concurrency situations have not been tested (we can check it when we have the trigger). |
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.
Closes #14720 #14725