Skip to content

store: skip chains in pool_size = 0 shards instead of panicking at startup (#6195)#6648

Open
cargopete wants to merge 1 commit into
graphprotocol:masterfrom
cargopete:fix/6195-pool-size-zero-startup
Open

store: skip chains in pool_size = 0 shards instead of panicking at startup (#6195)#6648
cargopete wants to merge 1 commit into
graphprotocol:masterfrom
cargopete:fix/6195-pool-size-zero-startup

Conversation

@cargopete

Copy link
Copy Markdown
Contributor

Summary

Fixes #6195. A node configured with pool_size = 0 for an unused shard (a
documented option from #3513) panics on startup:

WARN pool size for shard eth_chain is 0, ignoring this shard
thread 'main' panicked at node/src/store_builder.rs:203:
Creating the BlockStore works: InternalError("there is no pool for shard eth_chain")

store_builder correctly drops pool_size = 0 shards from the pool map, but
DieselBlockStore::new then iterates every chain — both configured chains
and all chains present in the database — and calls add_chain_store for each.
add_chain_store errors when a chain's shard has no pool, and that error is
.expect()ed in store_builder, killing startup. So pool_size = 0 is only
half-implemented: the pool is skipped, but the chains living in that shard are
not.

Fix

In both chain-enumeration loops in DieselBlockStore::new, skip chains whose
shard has no connection pool (logging a WARN) instead of calling
add_chain_store. This lets a node list all shards in config.toml but only
consume Postgres connections for the shards it actually uses, which is the
intent of pool_size = 0.

add_chain_store's contract is unchanged — it still errors when creating a
new chain in a shard with no pool, which remains a genuine misconfiguration.

Testing

  • cargo fmt, cargo clippy --all-targets, and cargo check --release clean
    for graph-store-postgres.

I was not able to add an automated test: DieselBlockStore::new and
ConnectionPool require a live multi-shard Postgres, so exercising the
multi-shard startup path isn't possible in a unit test. The change is small and
verified by inspection, but I'd appreciate a maintainer (or @0x19dG87) confirming
startup against a real multi-shard config with pool_size = 0 set on an unused
shard.

@lutter lutter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, just one small cleanup

chain.shard,
);
continue;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be better to do this filtering when we construct existing_chains rather than every time we iterate over it - essentially, this logic treats a chain with pool_size == 0 as not exsiting/configured.

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.

[Bug] Node Startup Fails When Using pool_size = 0 for Unused Shard Connections

2 participants