/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Avoid resetting state on rejected events #2025

Merged
merged 2 commits into from Mar 20, 2017

Conversation

Reviewers
No reviews
Assignees

@richvdh richvdh

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@richvdh @erikjohnston
@richvdh
Member

richvdh commented Mar 17, 2017

Presented as two commits: one which is a bit of a refactorathon, but should amount to a no-op functionally speaking, and a second which actually makes the change.

@richvdh richvdh assigned erikjohnston Mar 17, 2017

@erikjohnston
Owner

erikjohnston commented Mar 17, 2017

If we have a rejected state event that attempts to replace an older state, do we still see that older state for subsequent events?

richvdh added some commits Mar 17, 2017

@richvdh
Refactoring and cleanups 
A few non-functional changes:

* A bunch of docstrings to document types
* Split `EventsStore._persist_events_txn` up a bit. Hopefully it's a bit more
  readable.
* Rephrase `EventFederationStore._update_min_depth_for_room_txn` to avoid
  mind-bending conditional.
* Rephrase rejected/outlier conditional in `_update_outliers_txn` to avoid
  mind-bending conditional.
5068fb1
@richvdh
Avoid resetting state on rejected events 
When we get a rejected event, give it the same state_group as its prev_event,
rather than no state_group at all.

This should fix #1935.

Some checks were not successful

1 failing and 7 successful checks
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Dendron (Commit) — Build #1710 origin/rav/no_reset_state_on_rejections succeeded in 15 min
Details
@matrixbot
Sytest Postgres (Commit) — Build #2528 origin/rav/no_reset_state_on_rejections succeeded in 6 min 57 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2593 origin/rav/no_reset_state_on_rejections succeeded in 6 min 3 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
0c01f82
@richvdh
Member

richvdh commented Mar 17, 2017

If we have a rejected state event that attempts to replace an older state, do we still see that older state for subsequent events?

yes.
@erikjohnston
Owner

erikjohnston commented Mar 17, 2017

lgtm

@richvdh richvdh assigned richvdh and unassigned erikjohnston Mar 18, 2017

@richvdh richvdh merged commit 2e05f5d into develop Mar 20, 2017
7 of 8 checks passed

7 of 8 checks passed

Sytest Dendron (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1710 origin/rav/no_reset_state_on_rejections succeeded in 15 min
Details
Sytest Postgres (Commit) Build #2528 origin/rav/no_reset_state_on_rejections succeeded in 6 min 57 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2593 origin/rav/no_reset_state_on_rejections succeeded in 6 min 3 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@richvdh richvdh referenced this pull request Mar 20, 2017

Closed

Rejected events reset the room state #1935

@richvdh richvdh deleted the rav/no_reset_state_on_rejections branch Mar 22, 2017

This was referenced Apr 7, 2017

Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment