Skip to content

Commit ccc962f

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 ccc962f

8 files changed

Lines changed: 78 additions & 3 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/processor/EndTransactionProcessor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ public RemotingCommand processRequest(ChannelHandlerContext ctx, RemotingCommand
145145
msgInner.setPreparedTransactionOffset(requestHeader.getCommitLogOffset());
146146
msgInner.setStoreTimestamp(result.getPrepareMessage().getStoreTimestamp());
147147
MessageAccessor.clearProperty(msgInner, MessageConst.PROPERTY_TRANSACTION_PREPARED);
148+
MessageAccessor.clearProperty(msgInner, MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID);
148149
RemotingCommand sendResult = sendFinalMessage(msgInner);
149150
if (sendResult.getCode() == ResponseCode.SUCCESS) {
150151
deletePrepareMessage(result);

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
}

proxy/src/main/java/org/apache/rocketmq/proxy/service/client/ProxyClientRemotingProcessor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,12 @@ public RemotingCommand checkTransactionState(ChannelHandlerContext ctx,
6767
if (messageExt != null) {
6868
final String group = messageExt.getProperty(MessageConst.PROPERTY_PRODUCER_GROUP);
6969
if (group != null) {
70+
final String producerClientId = messageExt.getUserProperty(MessageConst.PROPERTY_TRANSACTION_PRODUCER_CLIENT_ID);
7071
CheckTransactionStateRequestHeader requestHeader =
7172
(CheckTransactionStateRequestHeader) request.decodeCommandCustomHeader(CheckTransactionStateRequestHeader.class);
7273
request.writeCustomHeader(requestHeader);
7374
request.addExtField(ProxyUtils.BROKER_ADDR, NetworkUtil.socketAddress2String(ctx.channel().remoteAddress()));
74-
Channel channel = this.producerManager.getAvailableChannel(group);
75+
Channel channel = this.producerManager.getAvailableChannel(group, producerClientId);
7576
if (channel != null) {
7677
channel.writeAndFlush(request);
7778
} else {

0 commit comments

Comments
 (0)