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

[v6] websocket background listener task #3206

Merged
merged 17 commits into from
Jan 25, 2024

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Jan 25, 2024

What was wrong?

v6 backport for #3179 and #3202

Todo:

Cute Animal Picture

20231220_091455

- Get rid of ``websockets.recv()`` calls in favor of a background task
  that listens for messages and puts them into appropriate queues /
  caches.

- This rids us of the need to have a ws_lock() around the ``recv()``
  call.

- Each provider has a listener task that error logs unless the user
  specifies an exception limit. In the case one is specified, exceptions
  will be raised and hault the listener task.

- In the (ideally) rare case that the subscription queue fills up to its
  max size, this will trigger an asycio.Event that will cause the
  listener task to wait until messages are consumed from the queue to
  resume listening.

(cherry picked from commit 2c947dd)
- Use INFO logging for communicating the state of the request processor
  caching.

- If the subscription message queue is in sync with the  websocket
  provider, refrain from INFO logging each message and instead log
  that we are in sync with the websocket message stream. If we go out
  of sync, log that we are out of sync and log the number of messages
  up to the point of sync.

- Rename some methods and classes to better reflect their purpose now
  that we no longer pull directly from the websocket but use a listener
  task and internally managed queue.

(cherry picked from commit 7de0980)
- Since we always have a listener background task that listens to the websocket now,
we should be more honest in the naming for this method. What it does is listen to
subscription messages that may or may not be synced with the websocket message stream.
Some of these may already be in the cache.

(cherry picked from commit 2a7f890)
(cherry picked from commit 82ffc87)
(cherry picked from commit a0b9964)
- Remove the ``endpoint_uri`` argument from the ``PersistentConenctionProvider`` base class, opting for defining it in the inheriting classes instead.
- Update the documentation around the ``WebsocketProviderV2`` class to reflect the current state + document ``PersistentConenctionProvider`` under a new section.
@fselmo fselmo force-pushed the websocket-background-listener-task-v6 branch from 80addb4 to 64e5c18 Compare January 25, 2024 18:27
@fselmo fselmo marked this pull request as ready for review January 25, 2024 18:28
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fselmo fselmo merged commit 62331bd into ethereum:v6 Jan 25, 2024
99 checks passed
fselmo added a commit that referenced this pull request Jan 25, 2024
@fselmo fselmo deleted the websocket-background-listener-task-v6 branch January 25, 2024 19:49
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