rabbitmq-stream-dotnet-client icon indicating copy to clipboard operation
rabbitmq-stream-dotnet-client copied to clipboard

Change reconnection logic in ReliableBase

Open ricsiLT opened this issue 3 years ago • 5 comments

In response to #150

ricsiLT avatar Jul 19 '22 09:07 ricsiLT

Thank you for your continued contributions. I just returned to work after some PTO, and @Gsantomaggio is out for a bit. We'll get to this when we can!

lukebakken avatar Aug 03 '22 22:08 lukebakken

@ricsiLT thanks for your patience, I'm reviewing this today.

lukebakken avatar Aug 08 '22 20:08 lukebakken

Yeah, test failures puzzled me too since I didn't understand why they wouldn't fail on main in the first place - chatted about it with @Gsantomaggio on Slack but he was busy with something else so I didn't get a clear answer about those tests :)

ricsiLT avatar Aug 09 '22 05:08 ricsiLT

@Gsantomaggio when you get back, let's discuss the remaining test failure. When running the ConsumeAfterKillConnectionShouldContinueToWork test we can see that many connections are killed at the end of the test. I'm wondering if the new reconnection logic creates many new connections?

rabbitmq-stream-dotnet-client-151-failure-log.txt

I'm going to look into how to enable logging while running tests. Not sure how to do that now.

lukebakken avatar Aug 09 '22 18:08 lukebakken

@ricsiLT Thanks for the PR! I didn't check the code, but there must be a bug somewhere. a lot of connections are recreated Screen Shot 2022-08-12 at 09 08 24

This is why the assert Assert.Equal(1, connectionsKilled); fails.

Gsantomaggio avatar Aug 12 '22 07:08 Gsantomaggio

@ricsiLT Thank you again for your contribution. I close this PR in favour of #163 !

Gsantomaggio avatar Sep 09 '22 12:09 Gsantomaggio