gkoszyk opened a new issue, #4134: URL: https://github.com/apache/iggy/issues/4134
Three tests land ignored in #4132, covering one defect at two layers plus one open decision. ### Tests to fix | Test | Location | |---|---| | `given_a_full_disk_when_the_offset_write_is_refused_then_persistence_should_not_fence` | `core/partitions/src/persistence.rs:1317` | | `given_a_full_disk_when_driving_persistence_then_only_the_partition_should_fence` | `core/partitions/src/iggy_partition.rs:15167` | | `given_a_divergent_prepare_when_submitting_then_the_partition_should_not_latch` | `core/partitions/src/iggy_partition.rs:15193` — **contested, see below** | ### Why they fail `write_consumer_offset` (`core/partitions/src/iggy_partition.rs:2695-2699`) calls `persistence.fail_operation` on **any** errno from `offset_storage.rs:159`'s bare `write_all_at`. `failure` is set-once. Its only `None` assignment is the constructor (`persistence.rs:526`); `truncate_from` and `reset_with_segments` both preserve it. `is_written_through` then reports **already-synced, already-acked** ops as unwritten — the first test fails on exactly that assertion, before the latch is even reached. `drive_persistence` raises `FatalCommit`, which the shard pump converts into a **whole-node shutdown** (`core/shard/src/lib.rs:7378-7384`). It runs on the replicated-apply path, so a disk-full condition is correlated: every replica of the topic latches within one op. Fail-closed is **correct for EIO and should stay** — Linux clears the dirty bit on writeback failure, so no retry is sound. Two things are not: - **ENOSPC/EDQUOT are not that class.** A refused write leaves prior bytes intact and nothing undefined. The same function already treats open failures as retriable (`offset_storage.rs:147-152`), so the classification is inconsistent one line up. - **The latch is per-partition; the escalation is per-node.** One offset record takes down every partition on the core, including topics with no persisted policy. `partitions.tombstone` is already used twice in #4092 (`partition_helpers.rs:687`, `:772`) as the right-sized fence. ### The contested test `submit_prepare_persistence` (`:1192-1193`) calls `persistence.fail(error)` for any non-`WouldBlock` error from `append`. Reviewers split: - The pre-checks (`op > head+1` and the checksum early-return) leave only a genuinely divergent prepare at an already-accepted op, where fail-closed is defensible → **delete the test**. - The same errno-classification fix covers both call sites → **un-ignore it and make it pass**. Recorded so the choice is explicit rather than implied by a missing test. ### Done when - [ ] Errno classified at `:2695`; latch reserved for EIO/unknown - [ ] `FatalCommit` for `StoreConsumerOffset` downgraded to a partition-local fence - [ ] First two `#[ignore]`s removed and passing - [ ] Decision recorded on the third: un-ignored and passing, or deleted with reasoning in the commit Found by an adversarial review of #4092. The first two correspond to a prior review finding that was marked resolved but was not fixed. -- 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]
