allthingssecurity opened a new pull request, #28197: URL: https://github.com/apache/camel/pull/28197
# Description [CAMEL-25793](https://issues.apache.org/jira/browse/CAMEL-25793) With `manualAcksEnabled=true`, `PahoMqtt5Consumer` acknowledges a message when its exchange completes, and only logs at DEBUG when the exchange fails. The option documentation says this "allows to rollback and retry the message in case of an error during routing in Camel". MQTT has no negative acknowledgement, though: the broker sends an unacknowledged PUBLISH again only when the client connects again to the same session (MQTT 5.0, section 4.4), and it never has more than the client's Receive Maximum unacknowledged QoS 1 / QoS 2 messages in flight to it (section 4.9). So: 1. a failed message was not retried while the connection lasted, and with a clean session (`cleanStart=true`, the default) never; 2. each failed message kept one place of the receive window. After `receiveMaximum` failed exchanges the broker sent the consumer nothing more. The route stayed started and nothing was logged above DEBUG. Change (in `PahoMqtt5Consumer`, `onFailure` of the manual acknowledgement): - with `cleanStart=false` the consumer disconnects and connects again, on one scheduled executor per consumer (the Paho client does not allow a disconnect from its callback thread), and subscribes again, so the broker sends the message again. The reconnects of failed exchanges in a row back off: the first waits the client's minimum automatic reconnect delay (1 s), each further one twice as long, up to `maxReconnectDelay`; a successful exchange resets it. Each such reconnect is logged at WARN, and a reconnect that fails is scheduled again with the next delay; - with a clean session, where the broker never sends the message again, the failed message is acknowledged, which frees its place in the window; the failure is left to the Camel error handler; - a consumer that uses a client set with the `client` option is not reconnected: the message stays unacknowledged as before, now with a WARN. What users can notice: with `cleanStart=false` a failed exchange makes the consumer reconnect after the backoff delay, and a message that always fails is delivered again and again, at most once per `maxReconnectDelay` (a Camel error handler such as a dead letter channel handles it). With a clean session a failed message is acknowledged. A reconnect also makes the broker send again the other messages that were not acknowledged yet (for example the ones still being routed by an asynchronous route). New section "Manual acknowledgements" in `paho-mqtt5-component.adoc` (and its catalog copy), and a note under its own `camel-paho-mqtt5` heading in the 4.24 upgrade guide. The description of the `manualAcksEnabled` option no longer says that a failed message is rolled back and retried in every case: it says what happens with and without a clean session (the component JSON, its catalog copy and the endpoint and component DSL are regenerated). Tests: - New `PahoMqtt5ManualAcksFailureTest` (4 tests, the real Paho client against an embedded Artemis MQTT broker from `camel-test-infra-artemis`, which camel-paho already uses; Artemis honours the client's Receive Maximum). Two test dependencies are added to the module pom for it. - `failedMessagesDoNotStopTheConsumer` (`receiveMaximum=2`; `fail-1` and `fail-2` fail once, `ok-3` and `ok-4` follow) fails without the change: `Expecting actual: [] to contain exactly ["ok-3", "ok-4"]`. - `failedMessageIsDeliveredAgainWithPersistentSession` (`cleanStart=false&sessionExpiryInterval=60`) fails without the change: `Expecting actual: ["ok-2"] to contain exactly in any order ["fail-1", "ok-2"]`. - `reconnectsAfterFailuresInARowBackOff` (a message that fails three times, `maxReconnectDelay=1500`): with the change the attempts are at least 1 s, 1.5 s and 1.5 s apart, and the success resets the backoff; without the change the message is never delivered again (`Expecting actual: [] to contain exactly ["fail-3"]`). - `acknowledgedMessagesKeepTheReceiveWindowOpen` passes with and without the change: four successful messages go through a window of 2, so the stall comes from the failures. - With the change the camel-paho-mqtt5 unit tests pass: 21 run, 0 failures, 0 errors, 0 skipped. How it was found: a TLA+ model of the manual acknowledgement protocol (a broker in-flight window, redelivery only on a new connection, the consumer's `onComplete` / `onFailure`). With the current code, "every published message reaches the route" is violated in 7 steps (two failed messages fill a window of 2). With the change it holds, and with a persistent session every message is also routed successfully without any message dropped. The test above reproduces the stall on the real client and broker. What is not changed: camel-paho (MQTT 3.1.1, deprecated) has the same `onFailure` and the same behaviour; it is not changed here. What was not run: the integration tests (they need a Mosquitto container); the full root build. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-25793](https://issues.apache.org/jira/browse/CAMEL-25793). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested camel-paho-mqtt5, including the formatter and import-sort plugins. No option changed; the catalog copy of the edited doc page is updated by hand. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 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]
