This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Use deltas to calculate current state deltas #3595
Merged
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
If we have a delta from the existing to new current state, then we can reuse that rather than manually working it out by fetching both lots of state.
richvdh
approved these changes
Jul 24, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, that looks like some code. I guess it might work?
synapse/storage/events.py
Outdated
if ev_id != current_state.get(key) | ||
} | ||
to_delete = [ | ||
key for key, ev_id in iteritems(existing_state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we do iterkeys
if we're not using the values? Or just in existing_state
Deferred[tuple[dict[(str,str), str]|None, dict[(str,str), str]|None]]: | ||
Returns a tuple of two state maps, the first being the full new current | ||
state and the second being the delta to the existing current state. | ||
If both are None then there has been no change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we clarify when the second is None but the first is not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(done)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Most of the time when we're updating current state during persist_events loop its a simple transition from an old state group to a new one, where we already have the delta between the two (from an event context). If we have the delta then we may as well reuse it, rather than pulling out the old and new current state and comparing them in app.