GitDagBundle: an interrupted clone leaves a version directory that can never check out and is never repaired
- Dominant language
- Python
- Stars
- 46.9k
- Forks
- 17.8k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 472
Description
### Apache Airflow Provider(s)
git
### Versions of Apache Airflow Providers
apache-airflow-providers-git 0.4.3 (also present in 0.3.1 and on main)
### Apache Airflow version
3.2.1 (code path unchanged on main)
### Operating System
Linux (containerized)
### Deployment
Official Apache Airflow Helm Chart
### Deployment details
`[dag_processor] dag_bundle_storage_path` points at a volume shared by the Dag processor and all worker pods.
### What happened
If a process is killed while `GitDagBundle` is cloning a bundle version, it leaves behind a directory that GitPython can open as a repository but that cannot resolve the configured branch or tag `tracking_ref`, and subsequent initialization attempts do not repair or replace it. Every later task pinned to that bundle version then fails during parsing with:
```
GitCommandError: Cmd('git') failed due to: exit code(1)
cmdline: git checkout
stderr: 'error: pathspec '' did not match any file(s) known to git'
```
The leftover directory looks like this:
- `versions//.git/` exists, with `HEAD`, `config`, `description`, `branches/`, `hooks/`, `info/` and a partially populated `objects/`
- `.git/refs/` contains no files and there is no `.git/packed-refs`
- `.git/HEAD` still contains the initial value `git clone` writes (`ref: refs/heads/`) rather than the cloned branch
- the working tree is empty (no Dag files)
`git clone` writes the `.git` skeleton and objects before it writes refs and checks out the working tree, so a process killed in that window leaves exactly this state. The bare mirror and the remote are both healthy - only the per-version clone is truncated.
The failure is not self-healing:
1. `_is_pruned_worktree()` treats "no `.git`" as a completed version, so a leftover `.git` makes it return `False`.
2. `_local_repo_has_version()` returns `False` for the truncated repo, so it does not take that shortcut either.
3. `_clone_repo_if_required()` only clones when `repo_path` does not exist. The path does exist, so it skips cloning and reopens the truncated repository.
4. `self.repo.git.checkout(self.tracking_ref)` then fails. It sits outside the `@retry` / `shutil.rmtree` block inside `_clone_repo_if_required()`, so nothing cleans the directory up and nothing retries.
Because the version directory is never repaired or replaced, tasks pinned to that bundle version continue to fail until the version directory is repaired or removed. A later bundle version may move newly scheduled tasks away from the poisoned version, but it does not fix the version itself. With shared bundle storage the blast radius is every worker, not one host.
One aggravating factor: the failure happens in `task_runner.parse()`, before the Dag is parsed, so task-level `on_failure_callback` / `on_retry_callback` never run. The failures are silent as far as Dag-level alerting is concerned.
### This may already be fixed by #71535
#71535 (open, closes #71388) adds a fetch before the checkout:
```python
if not self._has_version(self.repo, self.tracking_ref):
self.repo.remotes.origin.fetch()
self.repo.git.checkout(self.tracking_ref)
```
That looks like it would recover the state described here as well, for a different reason than it was written for:
- `_has_version(repo, tracking_ref)` cannot resolve `tracking_ref` in a repository with no refs, so it returns `False` and the fetch runs.
- `git clone` writes the remote configuration before it transfers objects, so an interruption during the object transfer can leave `remote.origin.url` configured. On such a leftover this can be confirmed with:
```
git -C versions/ config --get remote.origin.url
```
which points at the bundle's healthy local bare mirror.
- `origin` here is the bundle's local bare mirror, so the fetch is local and cheap, and it repopulates `refs/remotes/origin/*`, after which the checkout resolves.
If that is right, this issue is a second, unrelated trigger that #71535 happens to cover, and it would be worth an explicit regression test so the recovery is not lost in a later refactor. I would be glad to contribute that test to #71535 rather than open a competing fix.
One residual case #71535 would not cover: if the process is killed in the much narrower window before `git clone` writes the remote configuration, the leftover has no `origin` and `self.repo.remotes.origin` raises `AttributeError`. Whether that is worth handling is a separate question, and it is not what was observed here.
### What you think should happen instead
An interrupted clone should not permanently poison a bundle version. `GitDagBundle` should recover when an existing per-version repository cannot resolve the configured tracking ref, either by restoring the missing refs from its healthy local bare origin or, if that is not possible, by discarding and recreating the clone.
### How to reproduce
1. Configure a `GitDagBundle` with `[dag_processor] dag_bundle_storage_path` on storage that survives the process.
2. Start a task (or any bundle initialization) that has to clone a new version, and kill the process with `SIGKILL` a few seconds into the clone, while `objects/` is being populated and before the working tree appears.
3. Confirm the leftover state: `ls versions/` is empty, `git -C versions/ show-ref` prints nothing, `cat versions//.git/HEAD` still shows the `init.defaultBranch` value rather than the tracked branch.
4. Run any task pinned to that bundle version. It fails with the `pathspec ... did not match any file(s) known to git` error above, and keeps failing on every retry and every other task on that version.
Removing `versions/` by hand restores normal operation, because `_clone_repo_if_required()` then performs a real clone.
### Anything else
Preferred outcome, in order:
1. If #71535 does cover this, add an interrupted-clone regression test there: initialize a versioned bundle with a named branch such as `main` as `tracking_ref`, remove all loose and packed refs from `versions//.git`, leave HEAD pointing at a nonexistent branch, empty the working tree, verify that `origin` remains configured, then initialize the same version again and assert that both the expected version and the Dag files are restored. That test fails on `main` today and should pass with #71535. Happy to contribute it to that PR.
2. If #71535 turns out not to cover it, the narrow alternative is to remove `repo_path` before the existence check in `_clone_repo_if_required()` when it contains a `.git` with no references, so the normal clone path runs. A repository with no refs cannot resolve a branch or tag `tracking_ref`, so the normal initialization checkout cannot succeed until the refs are restored or the clone is recreated.
Moving `self.repo.git.checkout(self.tracking_ref)` inside the `@retry` / `rmtree` block of `_clone_repo_if_required()` would make any failing checkout discard the clone and retry once, which is broader and would cover more of these cases at once, but it also changes behaviour for failures that are not clearly recoverable.
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
### Code of Conduct
- [x] I agree to follow this project's Code of Conduct
Contributor guide
Research direction
Start by reading GitDagBundle’s _clone_repo_if_required(), _has_version(), and _local_repo_has_version() paths, then inspect the existing recovery change in #71535. Reproduce a versioned bundle with refs removed, an empty working tree, and origin preserved; done means reinitialization restores the tracking ref and DAG files without manual deletion.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- data-engineering
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100