Conversation
|
Claude finished @ss-es's task in 2m 2s —— View job Code Review: Migrate hash sequence key to bigint
This is a clean, well-scoped migration. The changes are consistent across all layers (SQL migration, Rust types, SQL cast expressions) and cover all four tables that reference Migration (V1501)The migration correctly:
I verified these are the only tables referencing One consideration for large tables: Rust ChangesAll
The SummaryLooks good to me. The PR is minimal and focused — it changes exactly what needs to change and nothing more. No issues found. |
There was a problem hiding this comment.
Code Review
This pull request updates the hash_id column and its references from 32-bit integers to 64-bit integers (BIGINT/i64) across the database schema and the Rust codebase to prevent sequence exhaustion. This includes database migrations, struct definitions, and serialization logic. A concern was raised regarding the database migration strategy; performing a direct ALTER COLUMN TYPE on a large table can cause significant downtime due to table rewrites and exclusive locks, so a multi-step migration approach should be considered for production environments.
I am having trouble creating individual review comments. Click here to see my feedback.
crates/espresso/node/api/migrations/postgres/V1501__increase_hash_id_to_bigint.sql (5)
Performing ALTER COLUMN TYPE on a primary key column in a table that has reached the SERIAL limit (approximately 2 billion rows) is a very heavy operation in PostgreSQL. It will require a full table rewrite and will hold an AccessExclusiveLock on the hash table for the duration of the operation, which could take several hours depending on the hardware and table size.
If this migration needs to be performed on a live production system with minimal downtime, consider a multi-step approach:
- Add a new
BIGINTcolumn. - Backfill the new column in batches.
- Create a trigger to keep the new column in sync.
- Swap the columns during a short maintenance window.
However, if the node is already failing due to the sequence exhaustion, this direct approach might be acceptable despite the downtime.
|
This seems to be extremely slow to run -- do not merge |
We hit the sequence limit for the hash table on a decaf node, and it's likely we'll hit the same limit on mainnet/other decaf nodes in the next few months. This PR switches the postgres id to a bigint, which should resolve the issue.
No sqlite migration is needed since that is already a u64.