search

maidsafe-archive / crust

Reliable p2p network connections in Rust with NAT traversal. One of the most needed libraries for any server-less / decentralised projects
BSD 3-Clause "New" or "Revised" License
956 stars 126 forks source link

Report about dropped messages #1134

Closed povilasb closed 5 years ago

povilasb commented 5 years ago

ETD + review: 2.5 days

The problem

  1. crust Service::send() will buffer given message.
  2. If message is not flushed withing 60 seconds, it is silently dropped.

Solution

  1. Change message expire time to 120 seconds.
  2. The actual buffering implementation is in socket-collection and it actually drops a whole queue rather than just one message. Message TX queue has many queues internally: one queue per priority. If first message of specific queue is expired, it will drop the whole queue. Make sure we only drop a single message instead. See: https://github.com/maidsafe/socket-collection/blob/master/src/out_queue.rs#L41
  3. Report/emit event about the dropped message to crust users.

Alternatives

We've been thinking of FIFO message queue with back pressure and message serialization to disk on shutdown. Hence, the whole dropped message notification procedure would be redundant. Maybe we should implement FIFO message queue right away?

ustulation commented 5 years ago

Message TX queue has many queues internally:

Based on this there would be just one queue moving f/w (as Crust is not supposed to deal with priorities any more). This size should be settable by the upper layer lib and back-pressure reported once it's full ?

douglascaetano commented 5 years ago

Closing this one as we decided the other way around, i.e., we will not notify message dropping anymore. See #1150 and other issues related to dynamic connections for more information.