Skip to content

Commit

Permalink
Use fresh context for each indexer (#97)
Browse files Browse the repository at this point in the history
Say we have some query that has no block requirement, so making the query deterministic will set a block hash based on the latest block for that indexer. If we need to try another indexer, then the context will be carried over and the next indexer may be queried with a block that is greater than its indexing status. This will result in unnecessary retries and penalties for many indexers that happen to be selected after the first for any query.

See also: graphprotocol/agora#31
  • Loading branch information
Theodus authored Dec 17, 2021
1 parent 82502d6 commit 2d720ff
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ actix-http = "=3.0.0-beta.14"
actix-web = "=4.0.0-beta.13"
async-trait = "0.1"
bs58 = "0.4"
cost-model = {git = "ssh://[email protected]/graphprotocol/agora.git", rev = "48bf329"}
cost-model = {git = "ssh://[email protected]/graphprotocol/agora.git", rev = "c7c9876"}
eventuals = "0.6"
futures = "0.3"
futures-util = "0.3"
Expand Down
18 changes: 18 additions & 0 deletions src/query_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,24 @@ impl<I: IndexerInterface + Clone + Send + 'static> QueryEngine<I> {
&[&deployment_ipfs],
|hist| hist.start_timer(),
);

// Since we modify the context in-place, we need to reset the context to the state of
// the original client query. This to avoid the following scenario:
// 1. A client query has no block requirements set for some top-level operation
// 2. The first indexer is selected, with some indexing status at block number `n`
// 3. The query is made deterministic by setting the block requirement to the hash of
// block `n`
// 4. Some condition requires us to retry this query on another indexer with an indexing
// status at a block less than `n`
// 5. The same context is re-used, including the block requirement set to the hash of
// block `n`
// 6. The indexer is seen as being behind and is unnecessarily penalized
//
// TODO: Avoid the additional cloning of the entire AST here, especially in the case
// where retries are necessary. Only the top-level operation arguments need to be reset
// to the state of the client query.
let mut context = context.clone();

let selection_result = self
.indexers
.select_indexer(
Expand Down
4 changes: 3 additions & 1 deletion src/query_engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,9 @@ impl IndexerInterface for TopologyIndexer {
let hash = capture.get(1).unwrap().as_str().parse::<Bytes32>().unwrap();
let number = blocks.iter().position(|block| block.hash == hash).unwrap();
if number > indexer.block(blocks.len()) {
todo!("block ahead of indexer")
return Err(
"Failed to decode `block.hash` value: `no block with that hash found`".into(),
);
}
}
Ok(IndexerResponse {
Expand Down

0 comments on commit 2d720ff

Please sign in to comment.