-
Notifications
You must be signed in to change notification settings - Fork 262
pyth-lazer-agent fixes #2771
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
pyth-lazer-agent fixes #2771
Conversation
… drain pending updates
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -95,6 +75,8 @@ impl RelayerSessionTask { | |||
.with_max_elapsed_time(None) | |||
.build(); | |||
|
|||
const FAILURE_RESET_TIME: Duration = Duration::from_secs(300); | |||
let mut first_failure_time = Instant::now(); | |||
let mut failure_count = 0; | |||
|
|||
loop { |
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.
Still doesn't use retry, I know. 😄
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. Can wait for one more approval to merge.
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.
Pull Request Overview
This PR refactors the relayer session and publisher to use a broadcast channel, adds a failure reset timer for backoff retries, fixes the default config file path, and ensures pending updates are drained.
- Switch mpsc channels to broadcast channels for fan-out to multiple relayer tasks
- Add a timer to reset backoff after 5 minutes without failures
- Update default config path and drain pending updates instead of clearing
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/relayer_session.rs | Converted to broadcast::channel , made session struct public, added backoff reset logic, handled lagged receiver errors |
src/main.rs | Changed default config file path to config/config.toml |
src/lazer_publisher.rs | Replaced custom RelayerSender with broadcast sender, drained pending updates before publishing |
Comments suppressed due to low confidence (4)
pyth-lazer-agent/src/relayer_session.rs:61
- Exposing all fields of
RelayerSessionTask
publicly can lead to misuse; consider adding a constructor or builder method and keeping fields private to maintain encapsulation.
pub struct RelayerSessionTask {
pyth-lazer-agent/src/relayer_session.rs:120
- [nitpick] The variable name
recv_result
is generic; consider renaming to something liketransaction_result
for clearer intent.
recv_result = self.receiver.recv() => {
pyth-lazer-agent/src/relayer_session.rs:134
- Add unit tests to simulate
RecvError::Lagged
andRecvError::Closed
paths to ensure logging and bail behavior works as expected.
broadcast::error::RecvError::Lagged(skipped_count) => {
pyth-lazer-agent/src/relayer_session.rs:78
- Consider adding tests to verify that the backoff strategy is reset after
FAILURE_RESET_TIME
has elapsed.
const FAILURE_RESET_TIME: Duration = Duration::from_secs(300);
@@ -104,6 +86,12 @@ impl RelayerSessionTask { | |||
return; | |||
} | |||
Err(e) => { | |||
if first_failure_time.elapsed() > FAILURE_RESET_TIME { |
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.
Currently failure_count
is only reset after the timer expires; on a successful WebSocket session you should also reset failure_count
(and possibly first_failure_time
) so that retries start fresh after each successful connection.
Copilot uses AI. Check for mistakes.
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.
A successful session will go indefinitely so I'll leave this as is.
Summary
Rationale
How has this been tested?