shlomitubul commented on PR #3721: URL: https://github.com/apache/celeborn/pull/3721#issuecomment-4649933404
Thanks for the thorough trace — addressed both points in the latest push (`922eba8`): 1. **Empty-partition edge case** (also Copilot's note): the helper no longer takes a separate empty-committed branch. `failed` is always computed as `requested − committed − empty`, and `COMMIT_FILE_EXCEPTION` is returned only when nothing committed **and** nothing is empty; otherwise `PARTIAL_SUCCESS`. So genuinely-empty partitions are never reported as failed (consistent with the success path), and the "all-empty, none committed" case no longer forces a recompute. Added two unit tests: empty-files-not-failed-when-nothing-committed, and all-empty-reports-no-failures. 2. **Concurrency / cancel doesn't interrupt** — added a comment at the snapshot making the ordering explicit: `cancel(true)` doesn't interrupt a running `CompletableFuture`, the committed/empty sets are append-only, and `failed` is computed (reading `committed`) *before* the `committed` snapshot — so a partition committing mid-window lands in both lists (safe over-report), never in neither. Calling out the load-bearing ordering so a future reorder doesn't reintroduce the silent-drop. Appreciate the review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
