alunduil / alunduil/zfs-replicate

execute() runs every task it is given regardless of order

未關閉
#653 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
enhancement
主要語言
Python
星號
24
分支
6
平均合併
3 小時 11 分鐘
30 天內合併 PR
49

描述

## Summary

`execute()` builds its per-action groups with a dict comprehension over
`itertools.groupby`, so if one filesystem's tasks contain the same action in two
non-adjacent runs, only the last run survives and the earlier tasks are dropped
without an error. Group in a way that cannot discard a run.

## Motivation

`zfs/replicate/task/execute.py:29`:

```python
action_tasks = {
action: list(action_tasks)
for action, action_tasks in itertools.groupby(filesystem_tasks, key=lambda x: x.action)
}
```

`groupby` yields one pair per *consecutive* run, so a `DESTROY, CREATE, DESTROY`
sequence yields three pairs and the second `DESTROY` key overwrites the first.
Confirmed against the real types:

```
input snapshots : ['s1', None, 's2']
survive execute : ['s2', None]
DROPPED : ['s1']
```

Nothing fails today. `generate()` never emits that shape: within a single
filesystem group it produces either one `CREATE` followed by sends, or one
contiguous block of destroys, because its two destroy branches are mutually
exclusive. Checked over the absent-remote, no-middles, middles,
remote-only, and mixed two-filesystem cases with `follow_delete` both ways, and
found no non-contiguous same-action group.

So this is a latent coupling rather than a live bug, and the cost is that it
constrains `generate()` silently. #651 declined to merge that function's two
destroy branches into a single `not middles or follow_delete` condition partly
because task position relative to the sends is load-bearing here; the comment at
`generate.py:42` records it. A future change that reorders emission would lose
tasks with no test failing and no log line.

## Scope

- Replace the dict comprehension so no run is discarded. Iterating the `groupby`
pairs directly is the closer fix: it needs no dict, and it preserves the true
interleaving. Accumulating runs into a list per action also stops the drop, but
it flattens `DESTROY, SEND, DESTROY` into all destroys then all sends, which is
a different execution order from the one `generate()` emitted.
- Keep the deepest-first sort at `execute.py:26` and the per-action dispatch.

## Acceptance criteria

- [ ] A filesystem whose tasks repeat an action in two non-adjacent runs executes
every task
- [ ] A test covers that shape directly, and fails if the dict comprehension
returns
- [ ] The order tasks execute in still follows the order `generate()` emitted them
- [ ] Existing `execute` tests pass unchanged

## Additional context

Found while refactoring `generate()` for #466 in #651, not from a user report, so
this is filed with the standard structure rather than the bug-report form, which
asks for a reproducing command and observed behaviour that do not exist here.

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。