-
Notifications
You must be signed in to change notification settings - Fork 965
Use HandleOrRuntime to allow alloydb/ethersdb to hold a custom runtime #1576
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
Changes from 4 commits
00adb4b
fda89ed
f94b1b7
ba20812
e6c8a57
01899bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,27 +2,29 @@ use std::sync::Arc; | |
|
|
||
| use ethers_core::types::{Block, BlockId, TxHash, H160 as eH160, H256, U64 as eU64}; | ||
| use ethers_providers::Middleware; | ||
| use tokio::runtime::Handle; | ||
| use tokio::runtime::{Handle, Runtime}; | ||
|
|
||
| use crate::primitives::{AccountInfo, Address, Bytecode, B256, U256}; | ||
| use crate::{Database, DatabaseRef}; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wtdcode would you explain why Clone has been removed? I'm slightly new in Rust and want to learn the technical concept behind this decision
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because runtime doesn’t implement Clone and it doesn’t make too much sense cloning the db. Or, you could wrap it with an Rc or Arc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks For now, I get an error (when cloning cacheDB) complains about ethersDB (as internal object of cacheDB) cannot be cloned. because inside separate threads, I need to write to db, I tested not cloning and use Arc/Mutex. but the performance was worse than cloning.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's not really make sense to clone
To your use case which is similar to mine, my suggestion is:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you mean |
||
| use super::utils::HandleOrRuntime; | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct EthersDB<M: Middleware> { | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| handle: Handle, | ||
| rt: HandleOrRuntime, | ||
| } | ||
|
|
||
| impl<M: Middleware> EthersDB<M> { | ||
| /// Create ethers db connector inputs are url and block on what we are basing our database (None for latest). | ||
| /// | ||
| /// Returns `None` if no tokio runtime is available or if the current runtime is a current-thread runtime. | ||
| pub fn new(client: Arc<M>, block_number: Option<BlockId>) -> Option<Self> { | ||
| let handle = match Handle::try_current() { | ||
| let rt = match Handle::try_current() { | ||
| Ok(handle) => match handle.runtime_flavor() { | ||
| tokio::runtime::RuntimeFlavor::CurrentThread => return None, | ||
| _ => handle, | ||
| _ => HandleOrRuntime::Handle(handle), | ||
| }, | ||
| Err(_) => return None, | ||
| }; | ||
|
|
@@ -31,29 +33,73 @@ impl<M: Middleware> EthersDB<M> { | |
| Some(Self { | ||
| client, | ||
| block_number, | ||
| handle, | ||
| rt, | ||
| }) | ||
| } else { | ||
| let mut instance = Self { | ||
| client: client.clone(), | ||
| client, | ||
| block_number: None, | ||
| handle, | ||
| rt, | ||
| }; | ||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(client.get_block_number()).ok()?, | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
| } | ||
|
|
||
| // Create a new EthersDB instance, with a provider and a block (None for latest) and a runtime. | ||
| // | ||
| // Refer to [tokio::runtime::Builder] how to create a runtime if you are in synchronous world. | ||
| // If you are already using something like [tokio::main], call EthersDB::new instead. | ||
| pub fn with_runtime( | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| runtime: Runtime, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the caller is responsible for creating a new runtime?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sense, we can add a new function and document the behavior. |
||
| ) -> Option<Self> { | ||
| let rt = HandleOrRuntime::Runtime(runtime); | ||
| let mut instance = Self { | ||
| client, | ||
| block_number, | ||
| rt, | ||
| }; | ||
|
|
||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
|
|
||
| // Create a new EthersDB instance, with a provider and a block (None for latest) and a handle. | ||
| // | ||
| // Refer to [tokio::runtime::Builder] how to create a runtime if you are in synchronous world. | ||
| // If you are already using something like [tokio::main], call EthersDB::new instead. | ||
| pub fn with_handle( | ||
| client: Arc<M>, | ||
| block_number: Option<BlockId>, | ||
| handle: Handle, | ||
| ) -> Option<Self> { | ||
| let rt = HandleOrRuntime::Handle(handle); | ||
| let mut instance = Self { | ||
| client, | ||
| block_number, | ||
| rt, | ||
| }; | ||
|
|
||
| instance.block_number = Some(BlockId::from( | ||
| instance.block_on(instance.client.get_block_number()).ok()?, | ||
| )); | ||
| Some(instance) | ||
| } | ||
|
|
||
| /// Internal utility function to call tokio feature and wait for output | ||
| #[inline] | ||
| fn block_on<F>(&self, f: F) -> F::Output | ||
| where | ||
| F: core::future::Future + Send, | ||
| F::Output: Send, | ||
| { | ||
| tokio::task::block_in_place(move || self.handle.block_on(f)) | ||
| self.rt.block_on(f) | ||
| } | ||
|
|
||
| /// set block number on which upcoming queries will be based | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| use tokio::runtime::{Handle, Runtime}; | ||
|
|
||
| // Hold a tokio runtime handle or full runtime | ||
| #[derive(Debug)] | ||
| pub(crate) enum HandleOrRuntime { | ||
| Handle(Handle), | ||
| Runtime(Runtime), | ||
| } | ||
|
|
||
| impl HandleOrRuntime { | ||
| #[inline] | ||
| pub(crate) fn block_on<F>(&self, f: F) -> F::Output | ||
| where | ||
| F: std::future::Future + Send, | ||
| F::Output: Send, | ||
| { | ||
| match self { | ||
| Self::Handle(handle) => tokio::task::block_in_place(move || handle.block_on(f)), | ||
| Self::Runtime(rt) => rt.block_on(f), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will panic if called within async execution context
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that’s intended. Passing a runtime mostly would be the sync code. If user is within async execution context, they should call new directly instead.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As stated above, if user is in async context, they should call new as #1557 suggests. If user is in sync context, they should provide a runtime, mostly current_thread runtime. If user intends to mix different contexts, they should be careful by themselves as it is known to be bad practice and cause problems hard to tackle. |
||
| } | ||
| } | ||
| } | ||

Uh oh!
There was an error while loading. Please reload this page.