adapter: Fix views synchronizer bugs #1249
Open
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
ethan-readyset
changed the title
readysetbot
force-pushed
the
Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
branch
from
2f1709d
to
e60957b
Compare
ethan-readyset
changed the title
readysetbot
force-pushed
the
Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
branch
2 times, most recently
from
4394921
to
f9ceca3
Compare
ethan-readyset
changed the base branch from
main
to
Id1465b9c660965c898932c783f5e47064a50a4b7
Previously, the views synchronizer only checked the server for views for queries that were in the "pending" state. This meant that if the migration handler set a query's state to "dry run succeeded" before the views synchronizer had a chance to check the server for a view, the query would be stuck in the "dry run succeeded" state forever, even if a view for the query did indeed exist already. This commit fixes the issue by having the views synchronizer check the server for views for queries in *either* the "pending" or "dry run succeeded" states. In order to prevent the views synchronizer from rechecking every query with status "dry run succeeded" over and over again, a "cache" has been added to the views synchronizer to keep track of which queries have already been checked. While working on this, I also noticed that it was possible for the following sequence of events to occur: - Migration handler sees that a query is pending and kicks off a dry run migration - Views synchronizer finds a view on the server for the same query and sets the status to "successful" - Migration handler finishes the dry run migration for the query and overwrites the status as "dry run succeeded" This could lead to a situation where a query that was previously (correctly) labeled as "successful" is moved back to the "dry run succeeded" state. To fix the issue, this commit updates the migration handler to only write the "dry run succeeded" status if the query's status is still "pending" after the dry run is completed. Release-Note-Core: Fixed a bug where queries that already had caches were sometimes stuck in the `SHOW PROXIED QUERIES` list Change-Id: Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
readysetbot
force-pushed
the
Ie5faa100158fc80c906d8ad5cb897d8a02a07be9
branch
from
f9ceca3
to
fb03811
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.
Previously, the views synchronizer only checked the server for views for
queries that were in the "pending" state. This meant that if the
migration handler set a query's state to "dry run succeeded" before the
views synchronizer had a chance to check the server for a view, the query
would be stuck in the "dry run succeeded" state forever, even if a view
for the query did indeed exist already.
This commit fixes the issue by having the views synchronizer check the
server for views for queries in either the "pending" or "dry run
succeeded" states. In order to prevent the views synchronizer from
rechecking every query with status "dry run succeeded" over and over
again, a "cache" has been added to the views synchronizer to keep track
of which queries have already been checked.
While working on this, I also noticed that it was possible for the
following sequence of events to occur:
migration
sets the status to "successful"
overwrites the status as "dry run succeeded"
This could lead to a situation where a query that was previously
(correctly) labeled as "successful" is moved back to the "dry run
succeeded" state. To fix the issue, this commit updates the migration
handler to only write the "dry run succeeded" status if the query's
status is still "pending" after the dry run is completed.
Release-Note-Core: Fixed a bug where queries that already had caches
were sometimes stuck in the
SHOW PROXIED QUERIESlist