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

Limit the number of events that can be created on a given room concurrently #1620

Merged
merged 3 commits into from Dec 12, 2016

Conversation

Projects
None yet
2 participants
Owner

erikjohnston commented Nov 10, 2016

No description provided.

erikjohnston added some commits Nov 10, 2016

synapse/util/async.py
+ max_count(int): The maximum number of concurrent access
+ """
+ self.max_count = max_count
+ self.key_to_defer = {}
@NegativeMjark

NegativeMjark Nov 10, 2016

Contributor
# key_to_defer is a map from the key to a 2 element list where
# the first element is the number of things executing
# the second element is a list of deferreds for the things blocked from executing.
synapse/util/async.py
+ def queue(self, key):
+ entry = self.key_to_defer.setdefault(key, [0, []])
+
+ if entry[0] >= self.max_count:
@NegativeMjark

NegativeMjark Nov 10, 2016 edited

Contributor
# If the number of things executing is greater than the maximum
# then add a deferred to the list of blocked items
# When one of the things currently executing finishes it will callback
# this item so that it can continue executing.
+ def _ctx_manager():
+ try:
+ yield
+ finally:
@NegativeMjark

NegativeMjark Nov 10, 2016

Contributor
# We've finished executing so check if there are any things blocked waiting to execute and start one of them
synapse/util/async.py
+ try:
+ entry[1].pop(0).callback(None)
+ except IndexError:
+ if entry[0] == 0:
@NegativeMjark

NegativeMjark Nov 10, 2016

Contributor
# If nothing else is executing for this key then remove it from the map
- if builder.type == EventTypes.Member:
- membership = builder.content.get("membership", None)
- target = UserID.from_string(builder.state_key)
+ with (yield self.limiter.queue(builder.room_id)):
@NegativeMjark

NegativeMjark Nov 10, 2016

Contributor
# Restrict the number of events that can happen concurrently in a room.
# Otherwise synapse may try to add many event simultaneously, forking the room state and making state resolution more expensive
# See (link to github issue) for more information.
synapse/handlers/message.py
@@ -50,6 +50,8 @@ def __init__(self, hs):
self.pagination_lock = ReadWriteLock()
+ self.limiter = Limiter(max_count=5)
@NegativeMjark

NegativeMjark Nov 10, 2016

Contributor

Comment on the choice of 5 here.

Contributor

NegativeMjark commented Nov 10, 2016

It'd be totes awesome to have some metrics on the total number of blocked and executing things for each limiter.

Contributor

NegativeMjark commented Dec 6, 2016

LGTM apart from the missing comments

@erikjohnston erikjohnston merged commit d53a80a into develop Dec 12, 2016

10 checks passed

Flake8 + Packaging (Commit) Build #2038 origin/erikj/concurrent_room_access succeeded in 1 min 18 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1105 origin/erikj/concurrent_room_access succeeded in 11 min
Details
Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #1944 origin/erikj/concurrent_room_access succeeded in 10 min
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #1989 origin/erikj/concurrent_room_access succeeded in 5 min 14 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #2069 origin/erikj/concurrent_room_access succeeded in 3 min 32 sec
Details
Unit Tests (Merged PR) Build finished.
Details

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

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