atsign-foundation / at_server

The software implementation of Atsign's core technology
https://docs.atsign.com
BSD 3-Clause "New" or "Revised" License
40 stars 12 forks source link

Eliminate all notification latency where possible, end-to-end #665

Closed gkc closed 2 years ago

gkc commented 2 years ago

Describe the bug Significant notification latency caused by

  1. Server attempts to forward notifications periodically - currently every 5 seconds
  2. Server closes outbound notification connections to other servers after sending them notifications. Thus every notification carries the full overhead of server-to-server authentication
  3. Server persists data before queueing the notification for delivery by the notifications ResourceManager

Expected behavior

  1. Keep the periodic job (necessary for retries) but nudge it into activity every time there is a new notification queued for delivery
  2. (a) Keep outbound notification connections open (b) make the notification connection pool into a Least-Recently-Used pool so that when we reach our configured limit on number of outbound connections, the LRU connection is closed
  3. Queue the notification for delivery before persisting it

Additional context Notifications are a critical component of the platform and should be delivered as close to instantaneously as possible.

gkc commented 2 years ago

Need an additional 5SP in sprint PR36 to (1) make the notification connection pool into an LRU pool (2) add unit tests

gkc commented 2 years ago

Ended up being 8 SP as in addition to making an LRU outbound client pool and adding unit tests, we found some additional issues with out-of-order notifications which needed another 2 days to diagnose and fix

gkc commented 2 years ago

Reminder @gkc follow up on https://github.com/atsign-foundation/at_server/pull/664#issuecomment-1114257096

gkc commented 2 years ago

Found another issue in final testing, related to trying to send notifications to at-signs that don't exist. Other than that, waiting for final review before merging to trunk