Fix: Reduce parsing overhead and improve performance when unpausing snapshots deployed to prod#3627
Merged
izeigerman merged 1 commit intomainfrom Jan 14, 2025
Conversation
izeigerman
force-pushed
the
fix-improve-performance-of-unpause-snapshots
branch
from
d9f682f to
499e2c3
Compare
…napshots deployed to prod
izeigerman
force-pushed
the
fix-improve-performance-of-unpause-snapshots
branch
from
499e2c3 to
d6f6cfa
Compare
tobymao
reviewed
Jan 14, 2025
tobymao
reviewed
Jan 14, 2025
| return self.change_category == SnapshotChangeCategory.FORWARD_ONLY | ||
|
|
||
| @property | ||
| def normalized_effective_from_ts(self) -> t.Optional[int]: |
Contributor
There was a problem hiding this comment.
should we make a mixin for these shared methods or no, not worth it?
Contributor
Author
There was a problem hiding this comment.
It's the only one. Didn't want to bother for now, but I'm open to reconsider
tobymao
reviewed
Jan 14, 2025
| snapshot.snapshot_id, | ||
| target_snapshot.snapshot_id, | ||
| ) | ||
| full_snapshot = snapshot.full_snapshot |
Contributor
There was a problem hiding this comment.
do you think it's worth hitting the cache here, or allowing full_snapshot to take in a cache?
Contributor
Author
There was a problem hiding this comment.
Maybe as a follow-up. It should happen rarely enough to not matter.
tobymao
approved these changes
Jan 14, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Unpausing snapshots that are deployed to prod involves fetching all snapshots that share the same version as the ones being deployed.
Before this change, fetching snapshots with the same versions involved full parsing of snapshot objects, including their models and queries. This caused a significant performance issue for projects with a high number of models, especially forward-only models.
With this update, SQLMesh will perform only a partial parsing when fetching snapshots with the same version, falling back to parsing the full snapshot only when absolutely necessary (which should be rare).
Additionally, this should also improve the performance of deleting expired snapshots, which relies on the same method to fetch snapshots with the same version.