hkc-8010 commented on PR #72787:
URL: https://github.com/apache/airflow/pull/72787#issuecomment-5630507047
This fixes a second leftover state that we hit in production, which is worth
recording here
because it is the case where the two competing approaches diverge.
Airflow 3.3.0, providers-git 0.4.x, long-lived worker pods with pod-local
bundle storage. A task
was marked failed from the UI while its worker was cloning the tracking
repo; SIGTERM landed 315
ms into the clone. `git clone` had written files into the working tree but
had not recorded them
in the index, so the checkout failed on untracked files rather than on a
missing ref:
```
git checkout master
error: The following untracked working tree files would be overwritten by
checkout:
.agents/instructions/airflow-patterns.md
<...>
Aborting
```
All 30 tasks that landed on that pod over the next 23 minutes failed at
`task_runner.py parse` -> `git.py:244 initialize` -> `git.py:215
_initialize`. Sibling pods in
the same ReplicaSet were fine, one completed 1,180 tasks the same day, and
it was the only worker
in three weeks with a 100% failure rate. It only cleared when the workers
rolled to a new
ReplicaSet.
Your change recovers this: the `GitCommandError` reaches the existing
`except (InvalidGitRepositoryError, GitCommandError)` handler,
`rmtree(repo_path)` runs, and the
retry produces a clean clone. #71535 does not, because fetching the missing
refs does nothing
about untracked files blocking the checkout. So the broader recovery here is
not just a
stylistic preference over the narrow fetch, it covers a state the fetch
cannot reach.
Happy to contribute a regression test for it if you want one in this PR:
clone with
`--no-checkout`, copy the tracked files into the working tree so they are
untracked, then assert
`initialize()` recovers. It fails on main for a different reason than your
existing test, so the
two are not redundant.
Separately I have opened #72922 and a PR for the prevention half, staging
the clone in a sibling
directory and renaming it into place so a killed clone cannot leave anything
at `repo_path` at
all. It is deliberately not a competing fix for the recovery, and the two
compose. We both touch
the clone block, so whoever merges second will need a trivial rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
