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

Connection handling needs work #4105

Closed
freimair opened this issue Mar 31, 2020 · 0 comments
Closed

Connection handling needs work #4105

freimair opened this issue Mar 31, 2020 · 0 comments

Comments

@freimair
Copy link
Contributor

freimair commented Mar 31, 2020

During #4047, I identified some issues with connection and message handling in the p2p part of bisq.

  • Messages are submitted to the connection asynchronously, thus, chances are that messages do not get sent because threads are abandoned, killed, time out, or a connection is closed before all message in queue are sent
  • definitely explains why we see nasty walls of exceptions on app shutdown quite frequently
  • concrete example: removeOfferMessages on shutdown may or may not be sent, depending on the timeouts and therefore on the performance of the host, network load, message load of the bisq app, seed node load, ...
  • it can happen for more crucial messages (messages are messages are messages, there are no priorities built into them)
  • might explain why we see messages getting lost

IMO:

  • I consider this a high priority task
  • given my >1,5 years experience with the p2p part of Bisq
  • the p2p message handling needs cleanup and refactoring, technology is outdated, changing stuff is a minefield, there is synchronization everywhere which immediately causes deadlocks on the slightest change, copy-and-paste spagetti code provides plenty of room for errors to hide in
  • however, I cannot provide a concrete issue # that will be fixed by working on this

Action plan:

  • start at the Connection.java class
    • implement a proper message queue for messages to be sent
    • let the connection decide on timing and threading for actually sending the message(s)
    • implement a proper connection shutdown process, move away from dropping anything instantly
  • gradually remove "external" message scheduling mechanics and thus,
    • eliminate unforeseen deadlock situations
    • confine timing issues to the Connection, where they can be (at least) handled somehow
    • track messages and see if they are actually sent

preliminary esitmates:

  • 3-5k USD for coding
  • plus testing
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

No branches or pull requests

1 participant