mattcasters opened a new issue, #7755: URL: https://github.com/apache/hop/issues/7755
## Context Raised during review of #7745 ([comment](https://github.com/apache/hop/pull/7745#issuecomment-5153839133)). This is **separate** from the done-flag ordering race fixed in #7742 / #7745. The reviewer correctly noted that `setDone()` and `putRowWait()` treat `getArray` differently, and that a failed enqueue can mean **lost rows** — the same *outcome* as #7742, but a different mechanism. They also stated it is **pre-existing** and **does not block** merging #7745. ## Concerns ### 1. Non-blocking offer in `setDone()` (reviewer note) - Full-batch path in `putRowWait`: timed `getArray.offer(inputBuffer, time, tu)` - Partial-batch path in `setDone`: non-blocking `getArray.offer(inputBuffer)` with return value discarded If `getArray` were full when `setDone()` runs, the final partial batch could be dropped silently. **Analysis under current design:** `getArray` capacity is `BATCHSIZE` (2), and there are only two physical buffers in the pool. While `inputBuffer != null` (required for a partial flush), at most one other buffer can sit in `getArray`, so there should always be room for the partial offer. The failure path looks **unreachable today**, but the asymmetry is fragile if buffer accounting changes later. ### 2. Stronger related bug: `putRowWait` ignores timed offer result When a full batch is published: ```java getArray.offer(inputBuffer, time, tu); // may return false on timeout inputBuffer = null; // always cleared return true; // always success to caller ``` If the timed offer fails under a slow consumer: - The full batch is **not** enqueued (data loss) - The physical buffer leaves the pool (capacity permanently reduced) - The producer is told the put succeeded This is more concrete than (1) under backpressure. ### 3. `setDone` does not clear `inputBuffer` after flush After a successful partial offer, `inputBuffer` / `putIndex` stay set. A second `setDone()` could re-offer the same buffer reference. ## Proposed work 1. **`setDone`:** use timed `offer` (aligned with put path) or block until space; on persistent failure, do not fail silently (log and/or avoid marking done / surface failure). Clear `inputBuffer` after a successful flush. 2. **`putRowWait`:** only clear `inputBuffer` when the timed offer succeeds; on failure return `false` and leave state retryable. 3. **Tests** (same style as #7745 white-box instrumentation where needed): - Full-batch path does not silently drop rows when offer fails / times out - `setDone` does not silently drop a partial batch if the queue is artificially full 4. Scope: `BlockingBatchingRowSet` + unit tests only. Default pipelines use `BlockingRowSet` unless `HOP_BATCHING_ROWSET=Y`. ## Related - #7742 — done-flag race (fixed) - #7745 — reorder `setDone` to flush before marking done ## Priority note Not P0 by default: (1) appears unreachable with the current 2-buffer design; (2) is real under backpressure but batching rowset is opt-in. Escalate if stress tests with `HOP_BATCHING_ROWSET=Y` show drops. -- 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]
