Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate reconnection logic #27

Closed
xforever1313 opened this issue Oct 9, 2017 · 2 comments
Closed

Investigate reconnection logic #27

xforever1313 opened this issue Oct 9, 2017 · 2 comments
Assignees
Labels

Comments

@xforever1313
Copy link
Owner

This is spun off of #23. We should figure out how to deal with reconnection. What if the parsing queue is very full, and we don't get a PONG back from the server before it has a chance to tell the watchdog? We'll reconnect for no good reason.

Per #23:

However, if several events are hanging for whatever reason, and we send a ping and wait for a pong, no amount of interrupts are going to save us before we time out. What we should do is when we reconnect via the watchdog thread, we should add that to the parsing queue. Then, once the parsing queue catches up, it will reconnect. When we reconnect, we'll restart the connection watchdog.

xforever1313 added a commit that referenced this issue Jan 21, 2018
Added IrcReconnector, which handles all of the reconnection logic if our watchdog timer times out.

Hopefully this will prevent the consistent joining/rejoining issue we've seen.

The only thing left to do is to add FitNesse tests.
@xforever1313
Copy link
Owner Author

Different approach:

Let's not handle pings and pongs on the string parsing queue. We don't want to connect/reconnect while the string parsing queue is busy. Instead, we should handle Pings and Pongs directly on the ReaderThread. This also means a user of Chaskis.Core doesn't have to add PingHandler and PongHandler to the list of handlers. This means PingHandler and PongHandler become internal instead of public.

This also means if we have multiple connections, we don't have to worry about juggling pings and pongs; everything is self-contained.

xforever1313 added a commit that referenced this issue Nov 1, 2020
Pings and Pongs now get resolved on the ReaderThread.

Should we add a test for this somehow????
@xforever1313 xforever1313 self-assigned this Nov 1, 2020
@xforever1313
Copy link
Owner Author

I'm going to close this issue. I think we know what the root-cause of all of this is, and we have a fix. We don't have any regression tests for this, but I'm not sure if its worth the trouble of adding one. Honestly, I wouldn't even know where to begin, there's a lot to add for something that can be clearly seen during code inspection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant