Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSSUE 1188]Fix the problem when more than one producer or consumer in the same process can trace only one #1275

Merged
merged 4 commits into from Jun 23, 2019

Conversation

ahuazhu
Copy link
Contributor

@ahuazhu ahuazhu commented Jun 19, 2019

What is the purpose of the change

Fix the bug when there are more than one producer (or consumer) , the Tracer can not show all trace infomation.

ISSUE 1188

Brief changelog

Set different ProducerGroup for different TraceProducer, the ProducerGroup will like _INNER_TRACE_PRODUCER-user-specified-producer-group

Verifying this change

Produce and consume message for a same topic in the same process, the console show both produce and consume information.
image

@coveralls
Copy link

coveralls commented Jun 19, 2019

Coverage Status

Coverage increased (+0.1%) to 50.589% when pulling 64051f2 on ahuazhu:trace_multi_topic into d66243c on apache:enchanced_msg_trace.

@ahuazhu ahuazhu changed the base branch from develop to enchanced_msg_trace June 19, 2019 07:38
@ahuazhu ahuazhu changed the title [ISSSUE 1188]Trace multi topic [ISSSUE 1188]Fix the problem when more than one producer or consumer in the same process can trace only one Jun 19, 2019
@zongtanghu
Copy link
Contributor

Hi @duhenglucky @ShannonDing @jonnxu please help to review this pr,together.


public AsyncTraceDispatcher(String traceTopicName, RPCHook rpcHook) {
public AsyncTraceDispatcher(String producerOrConsumerGroup, String traceTopicName, RPCHook rpcHook) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be better if the variable "producerOrConsumerGroup" change to be "group" name.

@@ -159,6 +161,10 @@ private DefaultMQProducer getAndCreateTraceProducer(RPCHook rpcHook) {
return traceProducerInstance;
}

private String makeGroupNameForTrace() {
return TraceConstants.GROUP_NAME_PREFIX + "-" + this.producerOrConsumerGroup;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above mentioned.

@@ -159,6 +161,10 @@ private DefaultMQProducer getAndCreateTraceProducer(RPCHook rpcHook) {
return traceProducerInstance;
}

private String makeGroupNameForTrace() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be better if makeGroupNameForTrace method name change to be genGroupNameForTrace

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All variables were renamed as your advice.

@zongtanghu
Copy link
Contributor

If users start the mutiple producer or consumer instances in the same process,the instance name is the same one.You can set different instance name in the start method when the traceProducer is started. @ahuazhu

@superheizai
Copy link
Contributor

LGTM

@ahuazhu
Copy link
Contributor Author

ahuazhu commented Jun 19, 2019

If users start the mutiple producer or consumer instances in the same process,the instance name is the same one.You can set different instance name in the start method when the traceProducer is started. @ahuazhu

If set different instance name for trace producer, multiple client instances will be created.

@@ -159,6 +161,10 @@ private DefaultMQProducer getAndCreateTraceProducer(RPCHook rpcHook) {
return traceProducerInstance;
}

private String genGroupNameForTrace() {
return TraceConstants.GROUP_NAME_PREFIX + "-" + this.group;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it can trace when the multi consumers or producers with the same group name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it can trace when the multi consumers or producers with the same group name?
Only the first traceProducer can trace, and the other will be start failed. If specify a instanceName for each group, it will create multi client instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonnxu
Thank you for the reminder. As far as I know, in RocketMQ two producers for different topics can't share the same ProduceGroup, and consumers also. But a consumer and a producer can have the same group. Unfortunately,the trace will failure in such scenes. To fix this problem, client type(consume or produce) will be appended to the trace-group to distinguish the consumer and producer.

 private String genGroupNameForTrace() {
        return TraceConstants.GROUP_NAME_PREFIX + "-" + this.group + "-" + this.type ;
    }

When construct AsyncTraceDispatcher.type will be set as "PRODUCE" or "CONSUME".

This PR has been updated.

And my test code as below:

public class SameProcessSameTopicTest {
    private static String topic1 = "TopicTest1";
    private static String topic2 = "TopicTest2";

//    private static final String CONSUMER_GROUP1 = "test_c_group1";
//    private static final String CONSUMER_GROUP2 = "test_c_group2";
//
//    private static final String PRODUCER_GROUP1 = "test_p_group1";
//    private static final String PRODUCER_GROUP2 = "test_p_group2";

    private static final String CONSUMER_GROUP1 = "test__group1";
    private static final String CONSUMER_GROUP2 = "test__group2";

    private static final String PRODUCER_GROUP1 = "test__group1";
    private static final String PRODUCER_GROUP2 = "test__group2";

    private static final String NAMR_SRV = "";

    public static void startConsume(final String id, final String group, final String topic) throws MQClientException {
        DefaultMQPushConsumer consumer = new DefaultMQPushConsumer(group, true);
        consumer.setConsumeFromWhere(ConsumeFromWhere.CONSUME_FROM_LAST_OFFSET);
        consumer.subscribe(topic, "*");
        consumer.setNamesrvAddr(NAMR_SRV);
        consumer.setInstanceName(id);
        final AtomicInteger i = new AtomicInteger(0);
        consumer.registerMessageListener(new MessageListenerConcurrently() {
            public ConsumeConcurrentlyStatus consumeMessage(List<MessageExt> msgs, ConsumeConcurrentlyContext context) {
                for (MessageExt msg : msgs) {
                    System.out.println(new Date() + ":" + new String(msg.getBody()) + " consume success! " + msg.getMsgId() + "  " + topic);
                }
                i.getAndAdd(msgs.size());

                System.err.println(id + " Consumed " + i.get() + " messages for " + topic);
                return ConsumeConcurrentlyStatus.CONSUME_SUCCESS;
            }
        });

        consumer.start();
    }

    public static void startProduce(String id, String group, String topic) throws MQClientException {
        DefaultMQProducer producer = new DefaultMQProducer(group,true);
        producer.setNamesrvAddr(NAMR_SRV);
        producer.start();
        producer.setInstanceName(id);
        AtomicInteger c = new AtomicInteger(0);
        for (int i = 0; i < 10 ;) {
            try {
                Message msg = new Message(topic /* Topic */, null /* Tag */,
                        ("trace message " + i).getBytes(RemotingHelper.DEFAULT_CHARSET) /* Message body */
                );
                producer.send(msg);
                Thread.sleep(50);
                i++;
                System.out.println(id + " send " + c.getAndIncrement() + " messages to " + topic);
            } catch (Exception e) {
//                e.printStackTrace();
            }
        }

    }


    public static void main(String[] args) throws InterruptedException, MQClientException {

        Thread t1 = new Thread() {
            @Override
            public void run() {
                try {
                    startConsume("C1", CONSUMER_GROUP1, topic1);
                } catch (MQClientException e) {
                    e.printStackTrace();
                }
            }
        };

        Thread t2 = new Thread() {
            @Override
            public void run() {
                try {
                    startConsume("C2", CONSUMER_GROUP2, topic2);
                } catch (MQClientException e) {
                    e.printStackTrace();
                }
            }
        };


        Thread t3 = new Thread() {
            @Override
            public void run() {
                try {
                    startProduce("P1", PRODUCER_GROUP1, topic1);
                } catch (MQClientException e) {
                    e.printStackTrace();
                }
            }
        };
//
        Thread t4 = new Thread() {
            @Override
            public void run() {
                try {
                    startProduce("P2", PRODUCER_GROUP2, topic2);
                } catch (MQClientException e) {
                    e.printStackTrace();
                }
            }
        };


        t1.start();
        t2.start();
        t3.start();
        t4.start();

        t1.join();
        t2.join();
        t3.join();
        t4.join();

        System.out.printf("Consumer Started.%n");
    }
} 

@duhenglucky
Copy link
Contributor

LGTM

@duhenglucky duhenglucky merged commit 0c5dae7 into apache:enchanced_msg_trace Jun 23, 2019
@chenyf3
Copy link

chenyf3 commented Dec 26, 2019

@duhenglucky when release?Thanks!

duhenglucky pushed a commit that referenced this pull request Jan 3, 2020
…in the same process can trace only one (#1275) (#1303)

* fix trace problem when multi produce/consumer in the same process

* uniform parameter manner

* variable rename

* consumer groups may be same with the producer group

Co-authored-by: zhengwen zhu <ahuazhu@gmail.com>
RongtongJin pushed a commit that referenced this pull request Jan 20, 2020
…in the same process can trace only one (#1275) (#1303)

* fix trace problem when multi produce/consumer in the same process

* uniform parameter manner

* variable rename

* consumer groups may be same with the producer group

Co-authored-by: zhengwen zhu <ahuazhu@gmail.com>
JiaMingLiu93 pushed a commit to JiaMingLiu93/rocketmq that referenced this pull request May 28, 2020
…in the same process can trace only one (apache#1275) (apache#1303)

* fix trace problem when multi produce/consumer in the same process

* uniform parameter manner

* variable rename

* consumer groups may be same with the producer group

Co-authored-by: zhengwen zhu <ahuazhu@gmail.com>
GenerousMan pushed a commit to GenerousMan/rocketmq that referenced this pull request Aug 12, 2022
…in the same process can trace only one (apache#1275) (apache#1303)

* fix trace problem when multi produce/consumer in the same process

* uniform parameter manner

* variable rename

* consumer groups may be same with the producer group

Co-authored-by: zhengwen zhu <ahuazhu@gmail.com>
pulllock pushed a commit to pulllock/rocketmq that referenced this pull request Oct 19, 2023
…in the same process can trace only one (apache#1275) (apache#1303)

* fix trace problem when multi produce/consumer in the same process

* uniform parameter manner

* variable rename

* consumer groups may be same with the producer group

Co-authored-by: zhengwen zhu <ahuazhu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants