Rangsh commented on PR #12081:
URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5633422540

   @DanielLeens thanks for the from-scratch re-review of `f17373ac5` and for 
catching the `delete`/`deleteAll` asymmetry — agreed this was the same 
silent/permanent durability-signal-loss pattern as Issue 1 from the previous 
round, just reached via retention pruning.
   
   Pushed **`28abb293d`** to close the blocker (and the Issue 2 write-through 
note):
   
   ### Issue 1 (blocking) — `FileMapStore.delete()`/`deleteAll()` must surface 
the same durability failure
   
   - Applied the identical `persistenceFailure(...)` pattern already used by 
`store()`/`storeAll()`:
     - `delete(key)` throws when `mapStorage.delete(key)` returns `false`
     - `deleteAll(keys)` throws when `mapStorage.deleteAll(keys)` returns a 
non-empty failure set
   - Same helper branches on `isAppendPermanentlyBlocked()` for the explicit 
\"restart this engine node\" message vs. the generic durability failure message.
   - Added three `FileMapStoreTest` cases mirroring the store coverage:
     - generic delete persistence failure
     - delete under permanent fail-close (asserts \"permanently fail-closed\" + 
\"restart\")
     - `deleteAll` with a non-empty failure set (asserts `deleteAll` + 
fail-closed message)
   
   Locally: `FileMapStoreTest` — 9 tests, all green.
   
   ### Issue 2 (non-blocking) — document write-through dependency
   
   - Added a class-level Javadoc on the `store`/`storeAll`/`delete`/`deleteAll` 
block noting that failure propagation assumes Hazelcast write-through 
(`write-delay-seconds=0`, the shipped default), and that write-behind would 
only log MapStore failures.
   
   ### Issue 3 (CI)
   
   - No production/test change that should affect the previously-red connector 
IT / Maven-Central flake jobs; happy to re-run those checks once CI picks up 
`28abb293d`.
   
   Glad to re-review whenever convenient — the hard design work from 
`f17373ac5` is unchanged; this is the mechanical same-file follow-up you 
described.


-- 
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]

Reply via email to