Reorg-pdpv0-cleanup-task

[snadrus]

This task ensures messages are on chain or otherwise alerts about the related rows for FUTURE clean-up processes to delete disk data.
Proposed as situation monitoring for theoretical problem #970

[LexLuthr]

We should rethink this approach. Whole Eth tx subsystem if forced to be 1 epoch only. it seems unfair that we only care about reverts for PDPv0. This task should be part of message tasks and should works on a much bigger scale. Reverts should also be checked by how forks are detected instead of missing message or receipts. Current method is not a reliable one.

[LexLuthr]

Reverting after reorg:

1. We should not check for whole chainFinality. That is too long. 5 epoch confidence is enough same as PoRep.
2. What we are doing is just clearing tables in this. We need to think in terms of pipeline instead of just table state.

• Create Message: Only affects data set tables
• Create and Add Message: Affects data tables, piece tables, index, IPNI records. We also need to be careful with pdp_pieceref table here. Due design choices there is no guarantee that ref now does not have a correct piece attached to it. Ideally, we should use delete flow (modified) for this to be reliable.
• Add Message: Same suggestion as above
• Remove Piece Message: This can be a simple retry in pipeline. Ideally, already dropped refs, index and IPNI should not cause an issue.

After all this we still have issues from newer messages landing on chain correctly after a reorg and taking up dataSet IDs and pieceIDs. We also need to account for that.

We need to understand impact of and guard against user mistakes like importing wrong snapshot, long running forks, bad build, lotus chain sethead etc.

What we really need to question is: Is this worth doing with so many conditions instead of just removing the bad data?

[snadrus]

@LexLuthr Since we are going for clean-up rather than re-run, most just erase the ask from DB. Delete is the most sensitive because our deleter must be chain-aware.

[LexLuthr]

We need to account for some missing chain reorg caveats here. I would also suggest checking what impact 8 hour time line has on proving subsystem. AFAIK, we would have faulted by then and this might not even be adding any value to it.

[snadrus]

@LexLuthr I see some conflicting things here. I thought this was a "clean-up" task, but If we reduce it to 5 blocks, we are making a guess about reorgs & not solving clean-up unless we "permit" failures for longer (more tables & states). If we run less frequently than proving period (30m?), they'll see a fault. A high volume of uploads cannot get chain walks one-by-one, so this must be a batched operation.

I see these options:
A: "Always right, always late"
This is just an 8h clean-up task so things get resolved eventually.

B: "Two bumps each"
Say we only validate 30m to 1h, those will see a fault, but will likely be past reorg. It'll also be a reasonable batch size to amortize a short-ish chain walk. Timing vs proving is weird.

C: "Fewest faults, gambling on definition of reorg"
We put this check at proof-time (30m?) and roll-back messages sent "5 blocks ago" that are missing. That minimizes faults.

These each do something different. I could see a world with A & partial-C being useful. You seem to not want me to build "A". Where do I go from here?

[ZenGround0]

To anchor the conversation a little just an FYI that lotus handles reorgs in a best effort way by re-inserting messages into the mpool whenever it notices a head change reverting its local chain:

https://github.com/filecoin-project/lotus/blob/master/chain/messagepool/messagepool.go#L1298

While it's true that things can go wrong and messages can be missed, note that the original motivation for issue #970 is purely theoretical at this point. We've never observed to my knowledge messages getting mysteriously missed after confirmation.

All of this to say -- its great to be paranoid and have something to handle long tail risk. But let's not boil the ocean.

[ZenGround0]

I'll finish later today. So far I like what I see

[snadrus]

I suspect this will happen one random day in a few months, and by the time we look at BetterStack traces, everything will have cleared up with the 7.5h Finality.
If it becomes a daily matter, we will have more than enough chances to define it to the best possible efficiency.

[ZenGround0]

At this point I don't see a reason to block.

[ZenGround0]

Oh I had missed this

Create and Add Message: Affects data tables, piece tables, index, IPNI records. We also need to be careful with pdp_pieceref table here. Due design choices there is no guarantee that ref now does not have a correct piece attached to it. Ideally, we should use delete flow (modified) for this to be reliable.
Add Message: Same suggestion as above

I agree this is a problem. My suggestion would be to only handle reorgs of creates and deletes (deletes were kind of the original scope of the issue). If you want to go with a full solution here then I think you do need to think about index, ipni and using a delete flow for DB state is probably the way to go.

[snadrus]

Oh I had missed this

Create and Add Message: Affects data tables, piece tables, index, IPNI records. We also need to be careful with pdp_pieceref table here. Due design choices there is no guarantee that ref now does not have a correct piece attached to it. Ideally, we should use delete flow (modified) for this to be reliable.
Add Message: Same suggestion as above

I agree this is a problem. My suggestion would be to only handle reorgs of creates and deletes (deletes were kind of the original scope of the issue). If you want to go with a full solution here then I think you do need to think about index, ipni and using a delete flow for DB state is probably the way to go.

This creates a whole new set of scope.

• For delete_reorg, those things should not be removed until the piece delete, which I hope we can have after the 1 hour when this runs, so that we don't need to re-add indexes and IPNI records.
• For create_reorg, we need to use a modified delete flow.

Again, until we have a working delete flow, I cannot modify it for either of these needs.

[rjan90]

Currently blocked by #1239 landing

[ZenGround0]

Currently blocked by #1239 landing

We're going to move forward landing this ~ as is and plug in full deletion pipeline integration in a follow on issue. The main reasons being that

1. As it stands this is a strict improvement over existing noop reorg handling
2. Reorgs clobbering state are currently theoretical. The motivating issue was hypothesizing this as a reason behind an existing error but no observations support it actually happening

[snadrus]

Currently blocked by #1239 landing

We're going to move forward landing this ~ as is and plug in full deletion pipeline integration in a follow on issue. The main reasons being that

1. As it stands this is a strict improvement over existing noop reorg handling
2. Reorgs clobbering state are currently theoretical. The motivating issue was hypothesizing this as a reason behind an existing error but no observations support it actually happening

I've switched to log-only under a dedicated logger for easy locating.

[beck-8]

from claude review

1. Main candidate SELECT blows up temp_file_limit (the actual production failure)

task_reorg_check.go:123-150

LEFT JOIN message_waits_eth mwe
  ON LOWER(TRIM(BOTH FROM mse.signed_hash)) = LOWER(TRIM(BOTH FROM mwe.signed_tx_hash))
...
NOT EXISTS (SELECT 1 FROM pdpv0_reorg_events re
            WHERE LOWER(re.tx_hash) = LOWER(TRIM(BOTH FROM mse.signed_hash)))

• LOWER(TRIM(...)) defeats the PK on message_waits_eth.signed_tx_hash → forced Hash Join across two large tables.
• WHERE filters on message_sends_eth (send_success, send_time, send_reason) have no usable index → seq scan on the whole table.
• On a real node this fails with ERROR: temporary file size exceeds temp_file_limit (1048576kB) (SQLSTATE 53400). Task retries forever and never makes progress.

2. Same anti-pattern in the "skipped" counter

task_reorg_check.go:279-291 — same LOWER(TRIM()) NOT EXISTS, same un-indexed WHERE. Only fires when lastEnd < epochLookback, which is exactly what happens once (1) keeps failing.

3. Redundant LOWER() on rollback lookups

task_reorg_check.go:462, 466, 470, 486, 490, 508, 526, 560, 575, 596 — wrap PK/UNIQUE columns in LOWER(), forcing seq scans. Hashes are written by tx.Hash().Hex() (already lowercase, no whitespace), so
LOWER()/TRIM() is unnecessary in every call site. Low-frequency code path, but should be cleaned up.

4. pdpv0_reorg_events index unusable as written

harmony/harmonydb/sql/20260509-pdpv0-reorg-chk.sql:10 — UNIQUE (tx_hash) is on the raw column, but every query compares LOWER(tx_hash). Index will never be used once the table grows.

Repro: node with hundreds of thousands of rows in message_sends_eth; query times out at 9–38 s with SQLSTATE 53400.