Skip to content

Commit 163dc4e

Browse files
author
wangjiahua.wjh
committed
[ISSUE #9791] Prefer original producer when checking transaction state
When multiple producers in the same group handle different topics, the transaction check may be routed to an unrelated producer that cannot determine the transaction state correctly. Fix: Record the original producer's clientId in the half message properties. During transaction check-back, prefer the channel matching that clientId. Fall back to round-robin if the original producer is offline. Changes: - MessageConst: add PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID - DefaultMQProducerImpl: write clientId into half message - ProducerManager: add getAvailableChannel(groupId, preferredClientId) - AbstractTransactionalMessageCheckListener: use new method - TransactionalMessageRocksDBService: use new method - ProducerManagerTest: 3 new test cases
1 parent 88709c5 commit 163dc4e

6 files changed

Lines changed: 63 additions & 2 deletions

File tree

broker/src/main/java/org/apache/rocketmq/broker/client/ProducerManager.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,30 @@ public Channel getAvailableChannel(String groupId) {
316316
return lastActiveChannel;
317317
}
318318

319+
/**
320+
* Get an available channel for the given group, preferring the producer that originally sent the message.
321+
* Falls back to round-robin if the preferred producer is not found or not available.
322+
*
323+
* @param groupId producer group
324+
* @param preferredClientId the clientId of the original producer (from half message properties), may be null
325+
* @return an available channel, or null if none found
326+
*/
327+
public Channel getAvailableChannel(String groupId, String preferredClientId) {
328+
if (preferredClientId != null) {
329+
ConcurrentMap<Channel, ClientChannelInfo> channelMap = groupChannelTable.get(groupId);
330+
if (channelMap != null) {
331+
for (Map.Entry<Channel, ClientChannelInfo> entry : channelMap.entrySet()) {
332+
if (preferredClientId.equals(entry.getValue().getClientId())
333+
&& entry.getKey().isActive() && entry.getKey().isWritable()) {
334+
return entry.getKey();
335+
}
336+
}
337+
}
338+
}
339+
// Fall back to round-robin selection
340+
return getAvailableChannel(groupId);
341+
}
342+
319343
public Channel findChannel(String clientId) {
320344
return clientChannelTable.get(clientId);
321345
}

broker/src/main/java/org/apache/rocketmq/broker/transaction/AbstractTransactionalMessageCheckListener.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public void sendCheckMessage(MessageExt msgExt) throws Exception {
6161
msgExt.setQueueId(Integer.parseInt(msgExt.getUserProperty(MessageConst.PROPERTY_REAL_QUEUE_ID)));
6262
msgExt.setStoreSize(0);
6363
String groupId = msgExt.getProperty(MessageConst.PROPERTY_PRODUCER_GROUP);
64-
Channel channel = brokerController.getProducerManager().getAvailableChannel(groupId);
64+
String producerClientId = msgExt.getUserProperty(MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID);
65+
Channel channel = brokerController.getProducerManager().getAvailableChannel(groupId, producerClientId);
6566
if (channel != null) {
6667
brokerController.getBroker2Client().checkProducerTransactionState(groupId, channel, checkTransactionStateRequestHeader, msgExt);
6768
} else {

broker/src/main/java/org/apache/rocketmq/broker/transaction/rocksdb/TransactionalMessageRocksDBService.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,8 @@ private void sendCheckMessage(MessageExt msgExt) {
234234
msgExt.setQueueId(Integer.parseInt(msgExt.getUserProperty(MessageConst.PROPERTY_REAL_QUEUE_ID)));
235235
msgExt.setStoreSize(0);
236236
String groupId = msgExt.getProperty(MessageConst.PROPERTY_PRODUCER_GROUP);
237-
Channel channel = brokerController.getProducerManager().getAvailableChannel(groupId);
237+
String producerClientId = msgExt.getUserProperty(MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID);
238+
Channel channel = brokerController.getProducerManager().getAvailableChannel(groupId, producerClientId);
238239
if (channel != null) {
239240
brokerController.getBroker2Client().checkProducerTransactionState(groupId, channel, checkTransactionStateRequestHeader, msgExt);
240241
} else {

broker/src/test/java/org/apache/rocketmq/broker/client/ProducerManagerTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,37 @@ public void testGetAvailableChannel() {
225225
assertThat(c).isNull();
226226
}
227227

228+
@Test
229+
public void testGetAvailableChannelWithPreferredClientId() {
230+
producerManager.registerProducer(group, clientInfo);
231+
when(channel.isActive()).thenReturn(true);
232+
when(channel.isWritable()).thenReturn(true);
233+
234+
// Match: preferred clientId matches registered producer
235+
Channel c = producerManager.getAvailableChannel(group, "clientId");
236+
assertThat(c).isSameAs(channel);
237+
}
238+
239+
@Test
240+
public void testGetAvailableChannelWithPreferredClientIdNotFound() {
241+
producerManager.registerProducer(group, clientInfo);
242+
when(channel.isActive()).thenReturn(true);
243+
when(channel.isWritable()).thenReturn(true);
244+
245+
// No match: falls back to round-robin (returns some channel from group)
246+
Channel c = producerManager.getAvailableChannel(group, "nonExistentClientId");
247+
assertThat(c).isNotNull(); // should fall back to round-robin
248+
}
249+
250+
@Test
251+
public void testGetAvailableChannelWithNullPreferredClientId() {
252+
producerManager.registerProducer(group, clientInfo);
253+
when(channel.isActive()).thenReturn(true);
254+
when(channel.isWritable()).thenReturn(true);
255+
256+
// null clientId: should behave exactly like original getAvailableChannel
257+
Channel c = producerManager.getAvailableChannel(group, null);
258+
assertThat(c).isNotNull();
259+
}
260+
228261
}

client/src/main/java/org/apache/rocketmq/client/impl/producer/DefaultMQProducerImpl.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,6 +1444,7 @@ public TransactionSendResult sendMessageInTransaction(final Message msg,
14441444
SendResult sendResult = null;
14451445
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_TRANSACTION_PREPARED, "true");
14461446
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_PRODUCER_GROUP, this.defaultMQProducer.getProducerGroup());
1447+
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID, this.mQClientFactory.getClientId());
14471448
try {
14481449
sendResult = this.send(msg);
14491450
} catch (Exception e) {

common/src/main/java/org/apache/rocketmq/common/message/MessageConst.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public class MessageConst {
5252
public static final String PROPERTY_TRANSACTION_PREPARED_QUEUE_OFFSET = "TRAN_PREPARED_QUEUE_OFFSET";
5353
public static final String PROPERTY_TRANSACTION_ID = "__transactionId__";
5454
public static final String PROPERTY_TRANSACTION_CHECK_TIMES = "TRANSACTION_CHECK_TIMES";
55+
public static final String PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID = "__TXN_PRODUCER_CID__";
5556
public static final String PROPERTY_INSTANCE_ID = "INSTANCE_ID";
5657
public static final String PROPERTY_CORRELATION_ID = "CORRELATION_ID";
5758
public static final String PROPERTY_MESSAGE_REPLY_TO_CLIENT = "REPLY_TO_CLIENT";

0 commit comments

Comments
 (0)