iht commented on PR #38603:
URL: https://github.com/apache/beam/pull/38603#issuecomment-5357684946
Hi @stankiewicz @ppawel,
I have pushed commit 2dc0ddc1622 to address the findings and review feedback
regarding the
`InvalidOperationException` observed during checkpoint timeouts on
standard flows.
### Summary of Changes
1. **Made Checkpoint NACK on Timeout Optional & Disabled by Default
(`withNackOnTimeout`)**:
- In Solace JCSMP, `settle(Outcome.FAILED)` is only supported on
redelivery flows on Solace broker 10.2.1+.
Calling `settle(Outcome.FAILED)` on standard consumer flows throws
`InvalidOperationException: Flow does not support
outcome FAILED`.
- Added `public static final boolean DEFAULT_NACK_ON_TIMEOUT = false;` and
`SolaceIO.Read.
withNackOnTimeout(boolean)` to make active NACKing an opt-in feature for
pipelines with supported flow configurations.
2. **Safe In-Memory Checkpoint Eviction (Default Behavior)**:
- When `nackOnTimeout = false` (the default), timed out checkpoints are
safely evicted from
`UnboundedSolaceReader`'s `pendingCheckpoints` map without calling
`msg.settle(...)`.
- This prevents memory leaks on long-running unfinalized checkpoints while
allowing the Solace broker to
redeliver unacknowledged messages upon session reconnection or flow rebind.
3. **Targeted Exception Handling on Settlement**:
- When `nackOnTimeout = true`, any `JCSMPException |
IllegalStateException` during message settlement is caught and logged with
message IDs (`applicationMessageId`, `ackMessageId`) rather than failing the
worker thread.
4. **Unit Tests Added**:
- `testCheckpointTimeoutDefault_EvictsWithoutNack`: Verifies that expired
checkpoints are evicted from memory without calling `settle()`.
- `testCheckpointTimeout_WithNackEnabled`: Verifies that
`msg.settle(Outcome.FAILED)` is asynchronously called
when `nackOnTimeout = true`.
- `testCheckpointTimeout_NackExceptionHandledGracefully`: Verifies that
`InvalidOperationException` is caught
and logged cleanly.
- `testWithNackOnTimeout`: Verifies that the configuration builder
propagates `withNackOnTimeout` correctly.
--
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]
