Skip to content

Commit 7ee5da8

Browse files
committed
store: take the max of source and destination history_blocks during graft
The graft finalization previously overwrote the destination's history_blocks with the source's value unconditionally. As a result, a destination subgraph manifesting `prune: never` (history_blocks = BLOCK_NUMBER_MAX) grafted from a `prune: auto` source silently inherited the source's much smaller retention window, contradicting the destination's manifest. Use max(src, dst) instead. The destination's retention is at least as long as the source's (otherwise the inherited copied data would be immediately eligible for pruning) while a destination that requests longer retention is no longer silently downgraded. Also expose `SubgraphStore::history_blocks` mirroring `set_history_blocks`, so the test can verify the post-graft retention value.
1 parent b3c4816 commit 7ee5da8

3 files changed

Lines changed: 82 additions & 6 deletions

File tree

store/postgres/src/deployment_store.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,11 +1591,11 @@ impl DeploymentStore {
15911591
}
15921592

15931593
let src_manifest_idx_and_name = src_deployment.manifest.template_idx_and_name()?;
1594-
let dst_manifest_idx_and_name = self
1595-
.load_deployment(dst.site.clone())
1596-
.await?
1597-
.manifest
1598-
.template_idx_and_name()?;
1594+
// Keep `dst_deployment` bound so we can read its
1595+
// `history_blocks` below; otherwise the chained
1596+
// `.manifest.template_idx_and_name()?` would consume it.
1597+
let dst_deployment = self.load_deployment(dst.site.clone()).await?;
1598+
let dst_manifest_idx_and_name = dst_deployment.manifest.template_idx_and_name()?;
15991599

16001600
// Copy subgraph data
16011601
// We allow both not copying tables at all from the source, as well
@@ -1672,10 +1672,25 @@ impl DeploymentStore {
16721672
info!(logger, "Rewound subgraph to block {}", block.number;
16731673
"time_ms" => start.elapsed().as_millis());
16741674

1675+
// Use the *maximum* of the source's and destination's
1676+
// `history_blocks` rather than overwriting the destination
1677+
// with the source's value. The destination has just
1678+
// received the source's full retained history, so its
1679+
// retention must be at least as long as the source's
1680+
// (otherwise the inherited data would be immediately
1681+
// eligible for pruning); but if the destination's
1682+
// manifest requests longer retention (notably
1683+
// `prune: never`, which yields `BLOCK_NUMBER_MAX`), that
1684+
// intent must be honoured. The previous unconditional
1685+
// overwrite silently downgraded `prune: never` children
1686+
// to whatever retention the parent used.
16751687
deployment::set_history_blocks(
16761688
conn,
16771689
&dst.site,
1678-
src_deployment.manifest.history_blocks,
1690+
src_deployment
1691+
.manifest
1692+
.history_blocks
1693+
.max(dst_deployment.manifest.history_blocks),
16791694
)
16801695
.await?;
16811696

store/postgres/src/subgraph_store.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,18 @@ impl Inner {
15201520
.await
15211521
}
15221522

1523+
/// Return the current `history_blocks` retention setting recorded in
1524+
/// the deployment's manifest. Mirrors [`Self::set_history_blocks`] and
1525+
/// is useful for tooling and tests that need to inspect the effective
1526+
/// retention after operations such as grafting.
1527+
pub async fn history_blocks(
1528+
&self,
1529+
deployment: &DeploymentLocator,
1530+
) -> Result<BlockNumber, StoreError> {
1531+
let site = self.find_site(deployment.id.into()).await?;
1532+
Ok(self.load_deployment(site).await?.manifest.history_blocks)
1533+
}
1534+
15231535
pub async fn load_deployment(
15241536
&self,
15251537
site: Arc<Site>,

store/test-store/tests/postgres/graft.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,3 +816,52 @@ fn graft_store_path_rejects_below_prune_floor() {
816816
Ok(())
817817
})
818818
}
819+
820+
/// Grafting must not silently shorten the destination's `history_blocks`
821+
/// retention.
822+
///
823+
/// Before the fix, `start_subgraph` unconditionally wrote the source
824+
/// subgraph's `history_blocks` into the destination, so a `prune: never`
825+
/// child grafted from a `prune: auto` parent silently inherited the
826+
/// parent's aggressive retention window. The fix takes the *maximum* of
827+
/// the two, so the destination keeps at least as much history as it has
828+
/// (the inherited copied data) but is never downgraded from a longer
829+
/// retention it explicitly requested.
830+
///
831+
/// The test setup matches that scenario: the destination is built by
832+
/// `test_store::create_subgraph`, whose manifest has `indexer_hints: None`
833+
/// and therefore reports `history_blocks = BLOCK_NUMBER_MAX` (the
834+
/// `prune: never` equivalent), while we manually lower the source's
835+
/// retention so old / new behaviour are observably different.
836+
#[test]
837+
fn graft_history_blocks_takes_max_of_parent_and_child() {
838+
run_test(|store, src| async move {
839+
// Lower the source's `history_blocks` to a small value so that
840+
// the "max vs overwrite" outcomes diverge measurably.
841+
let src_hb = 2;
842+
let reorg_threshold = 1;
843+
store
844+
.set_history_blocks(&src, src_hb, reorg_threshold)
845+
.await
846+
.expect("lowering source history_blocks");
847+
848+
let graft_id = DeploymentHash::new("grafted_history_blocks").unwrap();
849+
let dst =
850+
create_grafted_subgraph(&graft_id, GRAFT_GQL, src.hash.as_str(), BLOCKS[1].clone())
851+
.await
852+
.expect("graft succeeds");
853+
854+
let dst_hb = store
855+
.history_blocks(&dst)
856+
.await
857+
.expect("read dst history_blocks");
858+
assert_eq!(
859+
dst_hb, BLOCK_NUMBER_MAX,
860+
"graft must keep the destination's longer retention \
861+
(BLOCK_NUMBER_MAX = {BLOCK_NUMBER_MAX}) rather than overwriting it \
862+
with the source's smaller value ({src_hb})"
863+
);
864+
865+
Ok(())
866+
})
867+
}

0 commit comments

Comments
 (0)