Skip to content

Never read a tree the RPC peer lost as a deletion - #8776

Merged
jkschneider merged 1 commit into
mainfrom
rpc-lost-tree-is-not-a-deletion
Sep 26, 2026
Merged

jkschneider merged 1 commit into
mainfrom
rpc-lost-tree-is-not-a-deletion

Conversation

@jkschneider

@jkschneider jkschneider commented Sep 4, 2026 •

Copy link
Copy Markdown
Member

What happened

  • Running a composite Python migration recipe over real repositories deleted ordinary source modules that no recipe in the composite targets (for example tornado/template.py, src/textual/widget.py, xonsh/built_ins.py). Running every leaf recipe of the composite directly over the same files returns a tree for each of them; none returns None. The run that deleted them was also full of ref-table drift between the peers ("Received a reference to an object that was not previously sent" on the Java side, "Received reference to unknown object" on the Python side), and re-running the same recipe over the same LST with the facade ref-table fix from Fix the Python RPC facade's ref-table asymmetries #8768 produced no errors and no deletions.

Why a lost tree became a deletion

Every way the transport can fail to hand a tree back looked, to the scheduler, like the recipe having deleted it:

  • RewriteRpc.getObject answered null when the remote sent NO_CHANGE for an id this side holds no baseline for (a failed receive drops the baseline; the remote still records the object as delivered). RecipeRunCycle.flushBatch then returned that null as the file's new state, although no batch result had reported a deletion, and the file was removed.
  • The Python facade's _hub_pull_child_edit forgot a file whenever the child answered its pull with DELETE. A child answers DELETE both for a file its visitor deleted and for an id it no longer holds, and once the facade forgets the file the host's next GetObject sees DELETE too.

Fix

  • RewriteRpc.getObject: NO_CHANGE without a baseline is an IllegalStateException, never null (RpcReceiveQueue.peek added to look at the head state).
  • RecipeRunCycle.flushBatch: when a batch's results carry no deletion but the remote cannot produce the modified tree, the file is marked with the error through the same path a failed BatchVisit takes, instead of being dropped.
  • Python facade: a batch result that reports a deletion is dropped from the hub without a pull (hub_drop); _hub_pull_child_edit refuses a DELETE answer unless the caller allows it, which only the single-Visit path does, since there the child has no other way to report a deletion.

Tests

  • RewriteRpcTest.getObjectRejectsNoChangeWithoutABaseline
  • test_facade.py: test_batch_visit_drops_a_deleted_file_instead_of_pulling_it, test_hub_pull_refuses_a_child_that_lost_the_tree; the existing single-Visit deletion test now states that contract.

A source file was deleted from a repository when the peers' object tables
drifted apart mid-run, because every way the transport can fail to hand a
tree back looked, to the scheduler, like the recipe having deleted it:

- `RewriteRpc.getObject` returned null on a NO_CHANGE answer for an id this
  side holds no baseline for, and `RecipeRunCycle.flushBatch` returned that
  null as the file's new state even though no batch result reported a
  deletion.
- The Python facade's `_hub_pull_child_edit` forgot a file whenever the
  child answered its pull with DELETE, which the child also answers for an
  id it no longer holds; the host's next GetObject then saw DELETE.

NO_CHANGE without a baseline is now an error, a batch whose results carry
no deletion fails the file instead of dropping it when the remote cannot
produce the tree, and the facade only lets a pull delete a file on the
single-Visit path, where the child has no other way to report one; a batch
reports its deletions in its results and is dropped from the hub without a
pull.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Sep 4, 2026
@jkschneider
jkschneider merged commit 40506dd into main Sep 26, 2026
1 check passed
@jkschneider
jkschneider deleted the rpc-lost-tree-is-not-a-deletion branch September 26, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant