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

Optimise state resolution #1818

Merged
merged 8 commits into from Jan 18, 2017

Conversation

Projects
None yet
2 participants
Owner

erikjohnston commented Jan 17, 2017

No description provided.

erikjohnston added some commits Jan 13, 2017

synapse/event_auth.py
+ auth_types.append((EventTypes.Create, "", ))
+
+ if event.type == EventTypes.Member:
+ e_type = event.content["membership"]
@NegativeMjark

NegativeMjark Jan 17, 2017

Contributor

Maybe call this membership rather than e_type?

synapse/state.py
+ elif len(events) == 1:
+ unconflicted_state[key] = events[0].event_id
+
+ conflicted_state = new_conflicted_state
auth_events = {
@NegativeMjark

NegativeMjark Jan 17, 2017

Contributor

Doesn't this clobber the auth_events argument?

@NegativeMjark

NegativeMjark Jan 17, 2017

Contributor

Maybe the argument should be called auth_event_ids?

synapse/state.py
+ state_map):
+ new_conflicted_state = {}
+ for key, event_ids in conflicted_state.iteritems():
+ events = [state_map[ev_id] for ev_id in event_ids if ev_id in state_map]
@NegativeMjark

NegativeMjark Jan 17, 2017

Contributor

I wonder if state_map should be called state_events_by_id to make it clearer that it's a map from event_id to event.

synapse/state.py
+ ls.add(value)
+ if len(ls) == 2:
+ conflicted_state[key] = ls
+ unconflicted_state.pop(key, None)
@NegativeMjark

NegativeMjark Jan 17, 2017 edited

Contributor

Put some comments here to explain what's going on please.
(Also you could be super awful and use the unconflicted_state and the conflicted_state dict to track if you've seen the key before...)
((Which would mean that you could defer constructing the set until you had an actual conflict...))

@NegativeMjark

NegativeMjark Jan 17, 2017 edited

Contributor
for state_set in state_sets[1:]:
  for key, value in state_set.iter_items():
    # Check if there is an unconflicted entry for the state key.
    unconflicted_value = unconflicted_state.get(key)
    if unconflicted_value is None:
        # There isn't an unconflicted entry so check if there is a conflicted entry.
        ls = conflicted_state.get(key)
        if ls is None:
          # There wasn't a conflicted entry so haven't seen this key before.
          # Therefore it isn't conflicted yet.
          unconflicted_state[key] = value
        else:
          # This key is already conflicted, add our value to the conflict set.
          ls.add(value)
    elif unconflicted_value != value:
        # If the unconflicted value is not the same as our value then we have a new conflict.
        # So move the key from the unconflicted_state to the conflicted state.
        conflicted_state[key] = {value, unconflicted_value}
        unconflicted_state.pop(key, None)

erikjohnston and others added some commits Jan 17, 2017

@erikjohnston erikjohnston merged commit 15f0120 into develop Jan 18, 2017

7 of 8 checks passed

Sytest Dendron (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1385 origin/erikj/state_auth_splitout_split succeeded in 12 min
Details
Sytest Postgres (Commit) Build #2203 origin/erikj/state_auth_splitout_split succeeded in 6 min 55 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2271 origin/erikj/state_auth_splitout_split succeeded in 7 min 55 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

@erikjohnston erikjohnston deleted the erikj/state_auth_splitout_split branch Mar 29, 2017

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