Skip to content

perf(shadow-indexer): skip payload rewrite on canonical-only upsert - #4746

Open
wbj-cb wants to merge 1 commit into
mainfrom
perf-shadow-indexer-skip-payload-rewrite
Open

perf(shadow-indexer): skip payload rewrite on canonical-only upsert#4746
wbj-cb wants to merge 1 commit into
mainfrom
perf-shadow-indexer-skip-payload-rewrite

Conversation

@wbj-cb

@wbj-cb wbj-cb commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Skip the ~176KB JSONB payload rewrite when a shadow_blocks upsert only needs to stamp a canonical_hash.

Background

insert_rows upserts blocks with ON CONFLICT (number) DO UPDATE. Two kinds of conflict reach the DO UPDATE:

  1. New candidate hash for a height (reorg replacing the stored block) — genuinely new data.
  2. Same-hash redelivery carrying a canonical_hash — the block is unchanged; we only want to record that it became canonical.

Previously both cases assigned payload = EXCLUDED.payload unconditionally. In case (2) this reassigns the large JSONB payload to a byte-identical value, but Postgres still rewrites the row's TOAST chunks and emits full-page WAL for the tuple — hundreds of KB of write amplification just to set one hash column.

Change

Guard payload and created_at with a CASE keyed on whether the hash actually changed:

ON CONFLICT (number) DO UPDATE SET
  hash = EXCLUDED.hash,
  canonical_hash = EXCLUDED.canonical_hash,
  created_at = CASE WHEN shadow_blocks.hash <> EXCLUDED.hash
      THEN EXCLUDED.created_at ELSE shadow_blocks.created_at END,
  payload = CASE WHEN shadow_blocks.hash <> EXCLUDED.hash
      THEN EXCLUDED.payload ELSE shadow_blocks.payload END,
  updated_at = now()
WHERE shadow_blocks.hash <> EXCLUDED.hash
   OR EXCLUDED.canonical_hash IS NOT NULL

New hash (case 1): payload/created_at land wholesale, exactly as before.

  • Same-hash canonical stamp (case 2): payload is reassigned to itself. Postgres keeps the existing TOAST pointer, so no payload rewrite and no full-page WAL for the payload — only canonical_hash/updated_at change.

The WHERE clause is unchanged: redeliveries that neither change the hash nor add a canonical hash are still dropped.

Impact

  • Removes payload rewrite + full-page WAL amplification on the canonical-stamping path, which is the common steady-state conflict.
  • No behavioral change to stored data: identical rows result in both cases.

Testing

  • cargo test -p base-shadow-indexer-db — 4/4 pass
  • cargo clippy -p base-shadow-indexer-db — clean
  • cargo check -p base-shadow-indexer-db — clean

Notes

  • No schema/migration change; SQL-only.
  • Follow-up perf work (payload compression tuning, insert-only payload side table) is tracked separately.

A same-hash conflict only reaches DO UPDATE to stamp canonical_hash. Guard
payload/created_at with CASE so a same-hash conflict reassigns payload to
itself, keeping the existing TOAST pointer and avoiding a rewrite (plus
full-page WAL) of the ~176KB JSONB. New candidate hashes still land wholesale.
@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@depot-code-access

depot-code-access Bot commented Aug 27, 2026

Copy link
Copy Markdown

✅ All benchmarks green — 14 within ±2% (deterministic instruction counts). View run

Benchmark details (14)
Benchmark Base (target) Head (this PR) Δ instructions
batch_queue/drain/drain_cached_span_batches 242,027 242,027 +0.0%
batch_transaction/encode_in_place/encode_in_place 4,199,759 4,199,759 +0.0%
batch_transaction/temporary_frame_buffers/temporary_frame_buffers 8,408,350 8,408,350 +0.0%
flashblock_decode/decode/brotli 3,296,484 3,296,484 +0.0%
flashblock_decode/decode/plain_json 2,280,194 2,280,194 +0.0%
flz/compress_len/real_contract_call 43,148 43,148 +0.0%
flz/compress_len/synthetic_0 38,205 38,205 +0.0%
flz/compress_len/synthetic_1 54,682 54,682 +0.0%
flz/compress_len/synthetic_2 147,976 147,976 +0.0%
flz/data_gas 43,059 43,059 +0.0%
flz/tx_estimated_size 43,056 43,056 +0.0%
frame_parse/decode/single_4kib 1,031 1,031 +0.0%
frame_parse/parse_frames/few_large 1,053,062 1,053,062 +0.0%
frame_parse/parse_frames/many_small 154,763 154,763 +0.0%

@github-actions

Copy link
Copy Markdown
Contributor

Review: No issues found.

The SQL change is correct and well-targeted:

  • NULL safety: hash is NOT NULL in the schema, so shadow_blocks.hash <> EXCLUDED.hash never hits three-valued NULL logic. No edge case.
  • Case analysis: The three conflict scenarios (new hash, same-hash canonical stamp, same-hash redelivery without canonical) all produce identical stored outcomes to the prior version. The only difference is that case 2 now avoids a TOAST rewrite by self-assigning payload and created_at instead of reassigning from EXCLUDED.
  • WHERE clause: Unchanged and still correctly drops pure redeliveries (same hash, no canonical hash).
  • No block-production sensitivity: This touches the shadow indexer DB (observability for reorged blocks), not any block production, execution, or payload assembly path.

Clean optimization with no behavioral change.

@depot-code-access

Copy link
Copy Markdown

Base Std historical fork tests

Fork Result Passed Failed Skipped base/base base-anvil base-std
Beryl pass 616 0 13 d0e0d4f2 cecddfa5 4658f1b7
Cobalt pass 721 0 14 d0e0d4f2 fb00db40 e30b3421

View run

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.

2 participants