Linearize calls to _generate_user_id #3029
Conversation
| self._next_generated_user_id = ( | ||
| yield self.store.find_next_generated_user_id_localpart() | ||
| ) | ||
| with (yield self._generate_user_id_linearizer.queue(())): |
There was a problem hiding this comment.
surely we should have a test for self._next_generated_user_id is None inside the linearizer?
There was a problem hiding this comment.
Oops, yes! I've added a guard inside too, but kept the outer one for the fast path
| @@ -46,6 +46,10 @@ def __init__(self, hs): | |||
|
|
|||
| self.macaroon_gen = hs.get_macaroon_generator() | |||
|
|
|||
| self._generate_user_id_linearizer = Linearizer( | |||
There was a problem hiding this comment.
With the help of the twisted mailing list, I realised recently that our Linearizer is pretty inefficient, especially when you get lots of things queued up on it. In cases like this where you really just want to do a thing the first time we get to a bit of code, it's better to do it with a single Deferred that everything else hangs off:
if thing_result is None:
if thing_deferred is None:
thing_deferred = run_in_background(do_the_thing)
yield make_deferred_yieldable(thing_deferred)
(which is probably more-or-less equivalent to what some of our cache wrappers do)
having said that, if you feel disinclined to rewrite all this right now, I won't insist. Just I think we should bear it in mind for the future.
There was a problem hiding this comment.
Yup, we could use an ObservableDeferred here. I think with the reseed logic it becomes a non-trivial rewrite, so I think I'll punt until we have a chance to look at how we should be doing this properly.
(And cheekily remove a
cursor_to_dictcall)