Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
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
Deduplicate membership changes #700
Conversation
erikjohnston
assigned
NegativeMjark
Apr 6, 2016
|
Is there a reason to only deduplicate joins, or could this be used for all membership changes? |
NegativeMjark
commented on an outdated diff
Apr 6, 2016
| @@ -183,6 +185,44 @@ def update_membership( | ||
| third_party_signed=None, | ||
| ratelimit=True, | ||
| ): | ||
| + if action == Membership.JOIN: | ||
| + result = self.join_cache.get((room_id, target)) |
NegativeMjark
Contributor
|
|
@NegativeMjark PTAL |
|
LGTM |
NegativeMjark
and 1 other
commented on an outdated diff
Apr 7, 2016
| + | ||
| + with (yield linearizer.queue("test_key")): | ||
| + # do some work. | ||
| + | ||
| + """ | ||
| + def __init__(self): | ||
| + self.key_to_defer = {} | ||
| + | ||
| + @defer.inlineCallbacks | ||
| + def queue(self, key): | ||
| + current_defer = self.key_to_defer.setdefault(key, None) | ||
| + | ||
| + new_defer = defer.Deferred() | ||
| + self.key_to_defer[key] = new_defer | ||
| + | ||
| + def remove_if_current(_): |
NegativeMjark
Contributor
|
|
Maybe add a few comments explaining how the thing is supposed to work inside the |
NegativeMjark
commented on an outdated diff
Apr 7, 2016
| +class Linearizer(object): | ||
| + """Linearizes access to resources based on a key. Useful to ensure only one | ||
| + thing is happening at a time on a given resource. | ||
| + | ||
| + Example: | ||
| + | ||
| + with (yield linearizer.queue("test_key")): | ||
| + # do some work. | ||
| + | ||
| + """ | ||
| + def __init__(self): | ||
| + self.key_to_defer = {} | ||
| + | ||
| + @defer.inlineCallbacks | ||
| + def queue(self, key): | ||
| + current_defer = self.key_to_defer.setdefault(key, None) |
|
|
erikjohnston
added some commits
Apr 6, 2016
All checks have passed
erikjohnston
changed the title from
Deduplicate joins
to
Deduplicate membership changes
Apr 7, 2016
NegativeMjark
commented on an outdated diff
Apr 7, 2016
| + # resolved. | ||
| + # This all has the net effect of creating a chain of deferreds that | ||
| + # wait for the previous deferred before starting their work. | ||
| + current_defer = self.key_to_defer.get(key) | ||
| + | ||
| + new_defer = defer.Deferred() | ||
| + self.key_to_defer[key] = new_defer | ||
| + | ||
| + def remove_if_current(_): | ||
| + d = self.key_to_defer.get(key) | ||
| + if d is new_defer: | ||
| + self.key_to_defer.pop(key, None) | ||
| + | ||
| + new_defer.addBoth(remove_if_current) | ||
| + | ||
| + yield current_defer |
|
|
All checks have passed
NegativeMjark
commented on an outdated diff
Apr 7, 2016
| + # wait for the previous deferred before starting their work. | ||
| + current_defer = self.key_to_defer.get(key) | ||
| + | ||
| + new_defer = defer.Deferred() | ||
| + self.key_to_defer[key] = new_defer | ||
| + | ||
| + if current_defer: | ||
| + yield preserve_context_over_deferred(current_defer) | ||
| + | ||
| + @contextmanager | ||
| + def _ctx_manager(d): | ||
| + try: | ||
| + yield | ||
| + finally: | ||
| + d.callback(None) | ||
| + d = self.key_to_defer.get(key) |
|
|
All checks have passed
|
LGTM |
erikjohnston commentedApr 6, 2016
No description provided.