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

configure polling interval #437

Merged
merged 7 commits into from
Apr 23, 2024
Merged

Conversation

yash-atreya
Copy link
Member

Motivation

#290

Solution

Add methods to RpcClient and RootProvider to configure the polling interval.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@yash-atreya yash-atreya changed the title feat(rpc-client): add set_poll_interval method configure polling interval Apr 2, 2024
@yash-atreya
Copy link
Member Author

This wouldn't work. Making changes

inner.set_poll_interval(poll_interval);
self
} else {
todo!("TBD: Value is already shared. Cannot be mutated");
Copy link
Member

Choose a reason for hiding this comment

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

we could have the poll interval be an atomic number of seconds instead of a duration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if we have it as an atomic number, we would still need to use Arc::get_mut since RpcClientInner is wrapped in Arc.

Copy link
Member

Choose a reason for hiding this comment

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

atomics don't require a &mut reference to modify

Copy link
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

looking good. only nit

crates/rpc-client/src/client.rs Outdated Show resolved Hide resolved
@yash-atreya yash-atreya marked this pull request as ready for review April 22, 2024 23:36
Comment on lines +170 to +171
/// Note: Sets the poll interval to 250ms for local transports and 7s for remote transports by
/// default.
Copy link
Member

Choose a reason for hiding this comment

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

we could tweak this based in the chain, because 7s exceeds avg blocktime of a lot of chains.

the alloy_chains::Chain has a function for this.

so we could integrate this in eth_chainId call for example and track whether we've already set the poll_interval based on chain_id

@mattsse mattsse merged commit 5577b2e into alloy-rs:main Apr 23, 2024
18 checks passed
transport: t,
is_local,
id: AtomicU64::new(0),
poll_interval: if is_local { AtomicU64::new(250) } else { AtomicU64::new(7000) },
Copy link
Member

Choose a reason for hiding this comment

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

i'd make this 2s given the rise of L2s with faster blocktimes

ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
* feat(rpc-client): add set_poll_interval method

* fix: using with_poll_interval in RpcClient

* use AtomicU64 for poll interval

* PollerBuilder nits

* fix test

* store Ordering::Relaxed
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.

4 participants