Fix/38363 bridge stall #1325

codepitbull · 2025-12-08T14:35:45Z

Motivation

Resolves #38363

Changes

Under certai nconditions a bridge would not resume after losing its connection.

Copilot

Pull request overview

This PR fixes a critical bridge stall issue where MQTT bridges would fail to resume message forwarding after connection failures. The root cause involved inflight message markers not being properly cleared during reconnections, causing messages to be permanently stuck in persistence queues.

Key changes:

Introduces separate handling for initial connections vs reconnections to avoid duplicate message delivery
Adds removeAllInFlightMarkers method to persistence layer to reset stuck messages
Implements new callbacks (ResetAllInflightMarkersCallback, OnReconnectCallback) to coordinate reconnection state
Refactors drainQueue() to properly clear all stale state and reset inflight markers

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`legacy-modbus-adapter-minimal-config.xml`	Removed unused test resource file
`legacy-modbus-adapter-full-config.xml`	Removed unused test resource file
`ClientQueueMemoryLocalPersistence.java`	Implements new method to remove all inflight markers for a queue
`ClientQueuePersistenceImpl.java`	Adds persistence layer method to reset all inflight markers and notify waiting consumers
`ClientQueuePersistence.java`	Extends interface with removeAllInFlightMarkers method
`ClientQueueLocalPersistence.java`	Extends interface with removeAllInFlightMarkers method
`RemoteMqttForwarder.java`	Major refactoring to handle reconnection scenarios, distinguish initial connection from reconnection, and properly reset state
`BridgeMqttClient.java`	Adds logic to distinguish initial connection from reconnection using everConnected flag
`MqttForwarder.java`	Extends interface with new methods for flushing buffered messages and handling reconnection
`MessageForwarderImpl.java`	Implements callbacks to reset inflight markers and trigger message polling after reconnection
`FINDINGS_BRIDGE.md`	Documents root cause analysis of the bridge stall issue

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/RemoteMqttForwarder.java

github-actions · 2025-12-08T14:45:41Z

Test Results

483 files ±0 483 suites ±0 6m 15s ⏱️ +23s
4 053 tests ±0 4 050 ✅ ±0 3 💤 ±0 0 ❌ ±0
4 070 runs ±0 4 067 ✅ ±0 3 💤 ±0 0 ❌ ±0

Results for commit 672d008. ± Comparison against base commit eabcb94.

♻️ This comment has been updated with latest results.

github-actions · 2025-12-08T14:46:11Z

Coverage Report

Overall Project	65.29% `-0.35%`	✅
Files changed	13.97%	❌

File	Coverage
ClientQueueMemoryLocalPersistence.java	89.23% `-2.27%`	❌
RemoteMqttForwarder.java	70.17% `-16.82%`	❌
ClientQueuePersistenceImpl.java	52.99% `-2.45%`	❌
MessageForwarderImpl.java	9.56% `-23.77%`	❌
BridgeMqttClient.java	0.3% `-3.89%`	❌

caoccao

I hold one concern.

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/BridgeMqttClient.java

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/RemoteMqttForwarder.java

marregui · 2025-12-09T09:18:53Z

hivemq-edge/src/main/java/com/hivemq/bridge/MessageForwarderImpl.java

+                try {
+                    // Wait for all inflight markers to be reset before returning
+                    // This ensures onReconnect() will see clean queues when it triggers checkBuffers()
+                    Futures.allAsList(futuresBuilder.build()).get(30, TimeUnit.SECONDS);


the 30 secs could be configurable, or a constant.
Also, on timeout, i.e. when the markers are not removed, we still do onReconnect but the markets would not be reset, they would be stuck. Might this be a potential issue?, should the wait be longer? should the exception be propagated as a RT and prevent the reconnect?

Turned it into a constant.
Not really want to make it configurable.
The value is deliberately high. Normally the operation should finish in <100ms. If we ever reach the timeout the IO-layer is in trouble. And that's where the reconnect logic I added kicks in.

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/BridgeMqttClient.java

caoccao · 2025-12-09T11:19:05Z

hivemq-edge/src/main/java/com/hivemq/bridge/MessageForwarderImpl.java

+                }
+                final ImmutableList.Builder<ListenableFuture<Void>> futuresBuilder = ImmutableList.builder();
+                for (final String queueIdToReset : forwarderQueueIds) {
+                    futuresBuilder.add(queuePersistence.get().removeAllInFlightMarkers(queueIdToReset));


What if removeAllInFlightMarkers() is interrupted by Futures.allAsList(futuresBuilder.build()).get(30, TimeUnit.SECONDS)? Is there a rollback logic?

Normally the operation should finish in <100ms. If we ever reach the timeout the IO-layer is in trouble. And that's where the reconnect logic I added in the last commit kicks in.

caoccao · 2025-12-10T08:14:01Z

Better rebase to latest master. This branch has too much noise now.

sonarqubecloud · 2025-12-10T08:31:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

… connection problems

…warder.java Co-authored-by: Copilot <[email protected]>

1. forceReconnect() disconnects the MQTT client 2. BridgeMqttClient's addDisconnectedListener() detects the disconnect 3. Auto-reconnect kicks in with exponential backoff (1s min, 2min max) 4. On reconnect, addConnectedListener() fires 5. This calls drainQueue() → resetAllInflightMarkersCallback again 6. If persistence is now responsive, markers are cleared and messages flow

caoccao

LGTM

cla-bot bot added the cla-signed label Dec 8, 2025

codepitbull requested a review from Copilot December 8, 2025 14:36

Copilot started reviewing on behalf of codepitbull December 8, 2025 14:36 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

caoccao reviewed Dec 9, 2025

View reviewed changes

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/BridgeMqttClient.java Show resolved Hide resolved

hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/RemoteMqttForwarder.java Show resolved Hide resolved

marregui reviewed Dec 9, 2025

View reviewed changes

caoccao reviewed Dec 9, 2025

View reviewed changes

codepitbull and others added 10 commits December 10, 2025 11:29

Added a reproducer for the issue where a bridge would be stuck djring…

0c27a8e

… connection problems

Fix implemented

ac1734d

Fix round ... x

363f9f2

Remove reproducer

f63f471

Add data flow diagram for bridge

cf8e58b

Fix typo

87f16c4

Update hivemq-edge/src/main/java/com/hivemq/bridge/mqtt/RemoteMqttFor…

8327205

…warder.java Co-authored-by: Copilot <[email protected]>

Fix possible NPEs

8849193

Extract constant

672d008

codepitbull force-pushed the fix/38363-bridge-stall branch from 6983fa9 to 672d008 Compare December 10, 2025 10:35

codepitbull requested review from caoccao and marregui December 10, 2025 10:35

caoccao approved these changes Dec 10, 2025

View reviewed changes

Merge branch 'master' into fix/38363-bridge-stall

15218a2

codepitbull merged commit 1cccaeb into master Dec 11, 2025
6 of 7 checks passed

codepitbull deleted the fix/38363-bridge-stall branch December 11, 2025 14:44

Fix/38363 bridge stall #1325

Fix/38363 bridge stall #1325

Uh oh!

Conversation

codepitbull commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

caoccao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marregui Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

codepitbull Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

caoccao Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

codepitbull Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

caoccao commented Dec 10, 2025

Uh oh!

sonarqubecloud bot commented Dec 10, 2025

Quality Gate passed

Uh oh!

caoccao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Dec 8, 2025 •

edited

Loading

github-actions bot commented Dec 8, 2025 •

edited

Loading