chore: improve Migration() #3033
Conversation
| @@ -1331,6 +1339,7 @@ void Connection::DispatchFiber(util::FiberSocketBase* peer) { | |||
|
|
|||
| uint64_t prev_epoch = fb2::FiberSwitchEpoch(); | |||
| while (!builder->GetError()) { | |||
| DCHECK_EQ(socket()->proactor(), ProactorBase::me()); | |||
|
|
||
| DCHECK(!dispatch_fb_.IsJoinable()); | ||
|
|
There was a problem hiding this comment.
Did you check what happens with the dispatch fiber when it gets a message during migration?
There was a problem hiding this comment.
It shouldn't? The connection migrates, before it does it joins the dispatch_fb_. At this point we got no messages and we are the active fiber on the thread we are executing. Then we suspend on the current thread and we continue our execution on the thread we migrate. During that time the dispatch_fb_ should stay joined (that is, it couldn't be restarted) since no other operation can or should really wake it -- thinking of ACL's here(since the connection won't even show up when it gets removed by the thread local connection table and when it becomes available again on the thread local of the migrated thread it will be the active fiber - so no other fiber can wake up its queue).
Am I missing something?
There was a problem hiding this comment.
Theoretically... we check that no subscriptions are active when we migrate and no command is ongoin, so there shouldn't be any, right.
We have this dcheck to catch it if it does. Maybe there will be a reason do so in the future and we'll forget this is a potential bug 🙂
DCHECK