Skip to content

Conversation

@bluesliverx
Copy link

This bit me recently when channels were closed unexpectedly due to long running message retrieval on an API where client connections were closed suddenly. This apparently put the channels in a bad state (ChannelInvalidStateError("writer is None") to be exact), and therefore I had to check channels when retrieving them from the pool.

@bluesliverx bluesliverx force-pushed the master branch 2 times, most recently from 4c80718 to 38893c7 Compare February 16, 2021 15:22
@bluesliverx
Copy link
Author

Thanks @Sinkler for catching that. I found that I actually needed some more infrastructure around this to work properly since channels were attempting to reinitialize queues and exchanges on reopen. I've changed my commit to make sure things look good.

async def reopen(self) -> None:
# Clear out exchanges and queues when reopened
self._exchanges = defaultdict(set)
self._queues = defaultdict(set)
Copy link

@Sinkler Sinkler Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, we had some memory leaks with set so we replaced it by WeakSet in the __init__ method and now it's ok 🙃

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the info! I will make those same changes...

@bluesliverx
Copy link
Author

@mosquito the py35/py36-uvloop tox tests are failing, but I'm not sure what to do about those. They appear to be unrelated to my PR.

@bluesliverx
Copy link
Author

@mosquito I have updated the PR if you could take a look again. Thanks!

@mosquito mosquito added this to the 7.0 milestone Mar 10, 2021
@bluesliverx
Copy link
Author

Ran across this again, any chance of getting it merged @mosquito?

@mosquito
Copy link
Owner

@bluesliverx ouch, I should be write this comment earlier, but doesn't sorry. I completely do not understand why I will want to apply this example in real project when to be honest. When you want do not reconnecting when the underlay-connection faills you shouldn't use connect_robust, just use connect.

@bluesliverx
Copy link
Author

@mosquito, I do want the robust connection since I want it to re-established. However, I do not want the queues and exchanges to be declared again since some of them are exclusive and cannot be recreated by a new connection. I believe rabbit threw errors in that case.

As for why I need the channel.is_closed check, I'm not sure why the robust channel doesn't handle this internally, but I did find it was necessary when using pools. In certain cases such as clients closing connections mid-use, the channel was not robust enough to reconnect on its own and needed the check to reopen.

@IsaacFung
Copy link

Any chance of following up on this @mosquito? Thank you!

@mosquito
Copy link
Owner

@bluesliverx If some kind of check is required for reopening, it's a bug, you need to learn how to reproduce it, write a test and fix it.

As for exclusive consumers, Rabbit itself should release it after two failed heartbeats. It doesn't do this because for some reason it didn't receive a signal about closing the TCP socket from the client.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants