-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fselmo
merged 17 commits into
ethereum:v6
from
fselmo:websocket-background-listener-task-v6
Jan 25, 2024
Merged
[v6] websocket background listener task #3206
fselmo
merged 17 commits into
ethereum:v6
from
fselmo:websocket-background-listener-task-v6
Jan 25, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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)
(cherry picked from commit 23c4faa)
- 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)
(cherry picked from commit 1fc4071)
- 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 fe96e1e)
(cherry picked from commit 9ae0e13)
(cherry picked from commit 82ffc87)
(cherry picked from commit b9582d0)
(cherry picked from commit 48dfd4d)
(cherry picked from commit 51407e6)
(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
force-pushed
the
websocket-background-listener-task-v6
branch
from
January 25, 2024 18:27
80addb4
to
64e5c18
Compare
pacrob
approved these changes
Jan 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was wrong?
v6
backport for #3179 and #3202Todo:
Cute Animal Picture