Skip to content

Commit cccf293

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 cccf293

6 files changed

Lines changed: 75 additions & 2 deletions

File tree

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,33 @@ 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 (groupId == null) {
329+
return null;
330+
}
331+
if (preferredClientId != null) {
332+
ConcurrentMap<Channel, ClientChannelInfo> channelMap = groupChannelTable.get(groupId);
333+
if (channelMap != null) {
334+
for (Map.Entry<Channel, ClientChannelInfo> entry : channelMap.entrySet()) {
335+
if (preferredClientId.equals(entry.getValue().getClientId())
336+
&& entry.getKey().isActive() && entry.getKey().isWritable()) {
337+
return entry.getKey();
338+
}
339+
}
340+
}
341+
}
342+
// Fall back to round-robin selection
343+
return getAvailableChannel(groupId);
344+
}
345+
319346
public Channel findChannel(String clientId) {
320347
return clientChannelTable.get(clientId);
321348
}

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: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,44 @@ 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+
261+
@Test
262+
public void testGetAvailableChannelWithNullGroupId() {
263+
// null groupId with non-null preferredClientId should return null (no NPE)
264+
Channel c = producerManager.getAvailableChannel(null, "someClientId");
265+
assertThat(c).isNull();
266+
}
267+
228268
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,6 +1433,7 @@ public void sendOneway(Message msg, MessageQueueSelector selector, Object arg)
14331433
public TransactionSendResult sendMessageInTransaction(final Message msg,
14341434
final TransactionListener localTransactionListener, final Object arg)
14351435
throws MQClientException {
1436+
this.makeSureStateOK();
14361437
TransactionListener transactionListener = getCheckListener();
14371438
if (null == localTransactionListener && null == transactionListener) {
14381439
throw new MQClientException("tranExecutor is null", null);
@@ -1444,6 +1445,7 @@ public TransactionSendResult sendMessageInTransaction(final Message msg,
14441445
SendResult sendResult = null;
14451446
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_TRANSACTION_PREPARED, "true");
14461447
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_PRODUCER_GROUP, this.defaultMQProducer.getProducerGroup());
1448+
MessageAccessor.putProperty(msg, MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID, this.mQClientFactory.getClientId());
14471449
try {
14481450
sendResult = this.send(msg);
14491451
} catch (Exception e) {

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

Lines changed: 2 additions & 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";
@@ -173,5 +174,6 @@ public class MessageConst {
173174
STRING_HASH_SET.add(PROPERTY_CRC32);
174175
STRING_HASH_SET.add(PROPERTY_PRIORITY);
175176
STRING_HASH_SET.add(PROPERTY_LITE_TOPIC);
177+
STRING_HASH_SET.add(PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID);
176178
}
177179
}

0 commit comments

Comments
 (0)