-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
perf: Add support for multi get operation for database queries #2396
Open
netrome
wants to merge
37
commits into
master
Choose a base branch
from
2344-add-support-for-multi-get-operation-in-the-database
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
c77045f
feat: Multi-get with boxed iterators
netrome e66db04
feat: Multi get on bĺueprint
netrome 9eace87
feat: get_multi on RocksDB
netrome 8ba0c9d
feat: Use local fuel-vm
netrome 76b919b
feat: Implementation for structured storage
netrome 2333dfb
feat: Use multi-get
netrome 49994b9
feat: Messages impl
netrome 2fb9e88
feat: Don't rely on modified StorageInspect
netrome b2c2632
wip: Introduce specific StorageBatchInspect trait
netrome df63d48
wip: Use specific trait
netrome 4d5ca7d
feat: Use boxed iterator
netrome 36a50bb
feat: Use multi-get when getting full block
netrome 6275b36
feat: Use multi-get when getting coins
netrome e1e50fa
feat: Use multi-get when getting transactions
netrome f76d210
refactor: Rename multi_get -> get_batch
netrome 4feadfd
feat: Implement multi-get for more backends
netrome e141f56
chore: Clean up comments and add docstrings
netrome 8e3fd0f
chore: Add changelog entry
netrome 39664bb
fix: Take slice instead of Vec as input to function
netrome 52eca64
test: Assert the returned tx is in the expected place in get_full_blo…
netrome 5b0e975
fix: Typo
netrome bc955fd
fix: Typo
netrome 491c452
feat: Add metrics for RocksDB get_batch implementation
netrome 676a5d4
fix: Clippy
netrome 8b35460
fix: Remove stale TODO comment
netrome f697707
Revert "feat: Add metrics for RocksDB get_batch implementation"
netrome 3d0a71f
refactor: Return iterator over results in old multi_get function and …
netrome 5de80b7
perf: Only fall back to fetch off chain transactions if any result is…
netrome d7a30c8
fix: Whitespace
netrome f27bdbb
feat: Take Cow as keys in `KeyValueInspect::get_batch`
netrome 9ea8894
Merge branch 'master' into 2344-add-support-for-multi-get-operation-i…
netrome 2c2d222
Proposals to multi get PR (#2419)
xgreenx df33ef2
feat: Simplify `DatabaseCoins` port
netrome 79e42da
Another proposals to multi get PR (#2420)
xgreenx cbb6efc
feat: Don't require `Send` in BoxedIter
netrome 07f9b94
Merge branch 'master' into 2344-add-support-for-multi-get-operation-i…
netrome 75fedc7
fix: Cargo fmt
netrome File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,7 +177,7 @@ impl LowerHex for TxPointer { | |
} | ||
} | ||
|
||
#[derive(cynic::Scalar, Debug, Clone)] | ||
#[derive(cynic::Scalar, Debug, Clone, PartialEq, Eq)] | ||
pub struct HexString(pub Bytes); | ||
|
||
impl From<HexString> for Vec<u8> { | ||
|
@@ -194,7 +194,7 @@ impl Deref for HexString { | |
} | ||
} | ||
|
||
#[derive(Debug, Clone)] | ||
#[derive(Debug, Clone, PartialEq, Eq)] | ||
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. Same as above. |
||
pub struct Bytes(pub Vec<u8>); | ||
|
||
impl FromStr for Bytes { | ||
|
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
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
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -8,12 +8,11 @@ use crate::fuel_core_graphql_api::{ | |||||
use fuel_core_services::yield_stream::StreamYieldExt; | ||||||
use fuel_core_storage::{ | ||||||
iter::{ | ||||||
BoxedIter, | ||||||
BoxedIterSend, | ||||||
IntoBoxedIter, | ||||||
IntoBoxedIterSend, | ||||||
IterDirection, | ||||||
}, | ||||||
not_found, | ||||||
tables::Transactions, | ||||||
transactional::AtomicView, | ||||||
Error as StorageError, | ||||||
IsNotFound, | ||||||
|
@@ -65,6 +64,7 @@ use fuel_core_types::{ | |||||
use futures::Stream; | ||||||
use std::{ | ||||||
borrow::Cow, | ||||||
collections::BTreeMap, | ||||||
sync::Arc, | ||||||
}; | ||||||
|
||||||
|
@@ -141,29 +141,57 @@ impl ReadView { | |||||
pub fn transaction(&self, tx_id: &TxId) -> StorageResult<Transaction> { | ||||||
let result = self.on_chain.transaction(tx_id); | ||||||
if result.is_not_found() { | ||||||
if let Some(tx) = self.off_chain.old_transaction(tx_id)? { | ||||||
Ok(tx) | ||||||
} else { | ||||||
Err(not_found!(Transactions)) | ||||||
} | ||||||
self.off_chain.old_transaction(tx_id) | ||||||
} else { | ||||||
result | ||||||
} | ||||||
} | ||||||
|
||||||
pub async fn transactions( | ||||||
pub async fn transactions(&self, tx_ids: &[TxId]) -> Vec<StorageResult<Transaction>> { | ||||||
let transactions: Vec<_> = self | ||||||
.on_chain | ||||||
.transactions(tx_ids.iter().into_boxed()) | ||||||
.collect(); | ||||||
|
||||||
// Give a chance for other tasks to run. | ||||||
tokio::task::yield_now().await; | ||||||
|
||||||
if transactions.iter().any(|result| result.is_not_found()) { | ||||||
let on_chain_results = tx_ids.iter().enumerate().zip(transactions).collect(); | ||||||
|
||||||
self.extend_with_off_chain_results(on_chain_results).await | ||||||
} else { | ||||||
transactions | ||||||
} | ||||||
} | ||||||
|
||||||
pub async fn extend_with_off_chain_results( | ||||||
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.
Suggested change
|
||||||
&self, | ||||||
tx_ids: Vec<TxId>, | ||||||
on_chain_results: BTreeMap<(usize, &TxId), StorageResult<Transaction>>, | ||||||
) -> Vec<StorageResult<Transaction>> { | ||||||
// TODO: Use multiget when it's implemented. | ||||||
// https://github.com/FuelLabs/fuel-core/issues/2344 | ||||||
let result = tx_ids | ||||||
let off_chain_indexed_txids: Vec<_> = on_chain_results | ||||||
.iter() | ||||||
.map(|tx_id| self.transaction(tx_id)) | ||||||
.collect::<Vec<_>>(); | ||||||
// Give a chance to other tasks to run. | ||||||
.filter_map(|(indexed_tx_id, result)| { | ||||||
result.is_not_found().then_some(*indexed_tx_id) | ||||||
}) | ||||||
.collect(); | ||||||
|
||||||
let off_chain_results = off_chain_indexed_txids.iter().copied().zip( | ||||||
self.off_chain.old_transactions( | ||||||
off_chain_indexed_txids | ||||||
.iter() | ||||||
.map(|(_, tx_id)| *tx_id) | ||||||
.into_boxed(), | ||||||
), | ||||||
); | ||||||
|
||||||
let mut results = on_chain_results; | ||||||
results.extend(off_chain_results); | ||||||
|
||||||
// Give a chance for other tasks to run. | ||||||
tokio::task::yield_now().await; | ||||||
result | ||||||
|
||||||
results.into_values().collect() | ||||||
} | ||||||
|
||||||
pub fn block(&self, height: &BlockHeight) -> StorageResult<CompressedBlock> { | ||||||
|
@@ -178,7 +206,7 @@ impl ReadView { | |||||
&self, | ||||||
height: Option<BlockHeight>, | ||||||
direction: IterDirection, | ||||||
) -> BoxedIter<'_, StorageResult<CompressedBlock>> { | ||||||
) -> BoxedIterSend<'_, StorageResult<CompressedBlock>> { | ||||||
// Chain together blocks from the off-chain db and the on-chain db | ||||||
// The blocks in off-chain db, if any, are from time before regenesis | ||||||
|
||||||
|
@@ -191,12 +219,12 @@ impl ReadView { | |||||
.on_chain | ||||||
.blocks(Some(height), direction) | ||||||
.chain(self.off_chain.old_blocks(None, direction)) | ||||||
.into_boxed(), | ||||||
.into_boxed_send(), | ||||||
(false, IterDirection::Forward) => self | ||||||
.off_chain | ||||||
.old_blocks(Some(height), direction) | ||||||
.chain(self.on_chain.blocks(None, direction)) | ||||||
.into_boxed(), | ||||||
.into_boxed_send(), | ||||||
(false, IterDirection::Reverse) => { | ||||||
self.off_chain.old_blocks(Some(height), direction) | ||||||
} | ||||||
|
@@ -207,12 +235,12 @@ impl ReadView { | |||||
.off_chain | ||||||
.old_blocks(None, direction) | ||||||
.chain(self.on_chain.blocks(None, direction)) | ||||||
.into_boxed(), | ||||||
.into_boxed_send(), | ||||||
IterDirection::Reverse => self | ||||||
.on_chain | ||||||
.blocks(None, direction) | ||||||
.chain(self.off_chain.old_blocks(None, direction)) | ||||||
.into_boxed(), | ||||||
.into_boxed_send(), | ||||||
} | ||||||
} | ||||||
} | ||||||
|
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Are these changes needed? I can't seem to find a place where these additional derives are necessary.