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

fix(rpc)!: read from substream while streaming to check for interruptions #3548

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Nov 8, 2021

Description

  • fixes issue where long running streams continue long after client has dropped the stream
  • detects stream interruptions on server side and ends the RPC stream
  • (breaking change) send client protocol message back to explicitly end the stream
  • check for shutdown signal termination in client side while reading the stream

Motivation and Context

Streams should terminate as soon as they are not used, this was a particular problem when
streaming UTXOs to wallets and pruned nodes, when erroring out and retrying, new streams
would be established while the old streams still continued until termination.

The main trick was to read from the yamux stream just before writing to it without blocking if there
was nothing there. This allows yamux to receive the close stream message.

How Has This Been Tested?

New unit test
Manually, added code to forcefully error out of pruned sync after starting the stream. The node had another local node as a forced_sync_peer and so would continuously retry that peer. This caused many (saw up to 30) RPC sessions to accumulate and many concurrent streams. With these changes, the sessions used go between 1 and 0 for that peer.

@sdbondi sdbondi force-pushed the comms-rpc-check-stream branch from 119b7ff to 596302d Compare November 8, 2021 15:16
@sdbondi sdbondi force-pushed the comms-rpc-check-stream branch from 596302d to f4c63b5 Compare November 8, 2021 15:17
stringhandler
stringhandler previously approved these changes Nov 8, 2021
@aviator-app aviator-app bot merged commit 9194501 into tari-project:development Nov 8, 2021
@sdbondi sdbondi deleted the comms-rpc-check-stream branch November 9, 2021 04:51
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

Successfully merging this pull request may close these issues.

2 participants