Skip to content

Conversation

@Allenxuxu
Copy link

@Allenxuxu Allenxuxu commented Jun 25, 2021

@mreiferson mreiferson added the bug label Jul 5, 2021
@mreiferson mreiferson changed the title fix send closed channel when consumer reconnecting consumer: fix send on closed channel when reconnecting Jul 5, 2021
@mreiferson
Copy link
Member

Thanks for submitting this @Allenxuxu!

The Consumer exit code is unfortunately extremely convoluted and delicate, and I'm not convinced this fully addresses the issue. I think what needs to happen (at a high-level) is:

  1. Stop()
  2. Prevent reconnections (for both "lookup mode" and "direct connection") by ensuring that we exit lookupdLoop and complete any reconnect goroutines (like you've done here)
  3. Then we need to wait on all connections to close
  4. Stop handlers

Rejects the connection and quickly exits the reconnect loop after the consumer stops.

Signed-off-by: Allenxuxu <[email protected]>
@Allenxuxu
Copy link
Author

@mreiferson I'm sorry it took so long to reply.
I made a few changes:

  1. Check the ’r.stopFlag‘ before add a new connection to ’r.connections‘ , so we can prevent reconnections (for both "lookup mode" and "direct connection")
  2. Quickly exit the reconnect loop after consumer stops.

@mreiferson mreiferson self-requested a review October 24, 2021 20:39
@mreiferson
Copy link
Member

I'm not sure this is sufficient. Stop() doesn't close exitChan, so lookupdLoop() and any single-connection reconnects will still be racing. We need to ensure they all return before exit() gets called.

@Allenxuxu
Copy link
Author

so, maybe we can add stopChan to notify lookupdLoop and single-connection reconnects quickly exit.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants