You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by "qy-liuhuo (via GitHub)" <gi...@apache.org> on 2024/07/26 23:53:45 UTC

[PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

qy-liuhuo opened a new pull request, #10727:
URL: https://github.com/apache/inlong/pull/10727

   
   
   Fixes #10286 
   
   ### Motivation
   
   <!--Explain here the context, and why you're making that change. What is the problem you're trying to solve.-->
   
   update MQTT data source for Agent
   
   ### Modifications
   
   1. Update MQTT Task、Instance and source
   2. Update TaskConstants
   
   ### Verifying this change
   
   ![image](https://github.com/user-attachments/assets/9c457178-a235-40e1-b056-c72d6387cb8d)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "aloyszhang (via GitHub)" <gi...@apache.org>.
aloyszhang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694630633


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/reader/MqttReader.java:
##########
@@ -88,22 +77,22 @@ public MqttReader(String topic) {
      * @param jobConf
      */
     private void setGlobalParamsValue(InstanceProfile jobConf) {
-        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(JOB_MQTT_QUEUE_SIZE, 1000));
+        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(TaskConstants.TASK_MQTT_QUEUE_SIZE, 1000));
         instanceId = jobConf.getInstanceId();
-        userName = jobConf.get(JOB_MQTT_USERNAME);
-        password = jobConf.get(JOB_MQTT_PASSWORD);
-        serverURI = jobConf.get(JOB_MQTT_SERVER_URI);
-        topic = jobConf.get(JOB_MQTT_TOPIC);
-        clientId = jobConf.get(JOB_MQTT_CLIENT_ID_PREFIX, "mqtt_client") + "_" + UUID.randomUUID();
-        cleanSession = jobConf.getBoolean(JOB_MQTT_CLEAN_SESSION, false);
-        automaticReconnect = jobConf.getBoolean(JOB_MQTT_AUTOMATIC_RECONNECT, true);
-        qos = jobConf.getInt(JOB_MQTT_QOS, 1);
-        mqttVersion = jobConf.getInt(JOB_MQTT_VERSION, MqttConnectOptions.MQTT_VERSION_DEFAULT);
-
+        userName = jobConf.get(TaskConstants.TASK_MQTT_USERNAME);
+        password = jobConf.get(TaskConstants.TASK_MQTT_PASSWORD);
+        serverURI = jobConf.get(TaskConstants.TASK_MQTT_SERVER_URI);
+
+        // topic = jobConf.get(TaskConstants.TASK_MQTT_TOPIC);

Review Comment:
   remove this line if not used.



##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/reader/MqttReader.java:
##########
@@ -88,22 +77,22 @@ public MqttReader(String topic) {
      * @param jobConf
      */
     private void setGlobalParamsValue(InstanceProfile jobConf) {
-        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(JOB_MQTT_QUEUE_SIZE, 1000));
+        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(TaskConstants.TASK_MQTT_QUEUE_SIZE, 1000));
         instanceId = jobConf.getInstanceId();
-        userName = jobConf.get(JOB_MQTT_USERNAME);
-        password = jobConf.get(JOB_MQTT_PASSWORD);
-        serverURI = jobConf.get(JOB_MQTT_SERVER_URI);
-        topic = jobConf.get(JOB_MQTT_TOPIC);
-        clientId = jobConf.get(JOB_MQTT_CLIENT_ID_PREFIX, "mqtt_client") + "_" + UUID.randomUUID();
-        cleanSession = jobConf.getBoolean(JOB_MQTT_CLEAN_SESSION, false);
-        automaticReconnect = jobConf.getBoolean(JOB_MQTT_AUTOMATIC_RECONNECT, true);
-        qos = jobConf.getInt(JOB_MQTT_QOS, 1);
-        mqttVersion = jobConf.getInt(JOB_MQTT_VERSION, MqttConnectOptions.MQTT_VERSION_DEFAULT);
-
+        userName = jobConf.get(TaskConstants.TASK_MQTT_USERNAME);
+        password = jobConf.get(TaskConstants.TASK_MQTT_PASSWORD);
+        serverURI = jobConf.get(TaskConstants.TASK_MQTT_SERVER_URI);
+
+        // topic = jobConf.get(TaskConstants.TASK_MQTT_TOPIC);

Review Comment:
   remove this line if not used.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694562488


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/task/MqttTask.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.agent.plugin.task;
+
+import org.apache.inlong.agent.conf.InstanceProfile;
+import org.apache.inlong.agent.conf.TaskProfile;
+import org.apache.inlong.agent.constant.CycleUnitType;
+import org.apache.inlong.agent.constant.TaskConstants;
+import org.apache.inlong.agent.utils.AgentUtils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
+
+public class MqttTask extends AbstractTask {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(MqttTask.class);
+
+    private String topic;
+
+    private boolean isAdded = false;
+
+    public static final String DEFAULT_MQTT_INSTANCE = "org.apache.inlong.agent.plugin.instance.MqttInstance";
+
+    private final DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern("yyyyMMddHH");
+
+    @Override
+    public boolean isProfileValid(TaskProfile profile) {
+        if (!profile.allRequiredKeyExist()) {
+            LOGGER.info("task profile needs all required key");
+            return false;
+        }
+        if (!profile.hasKey(TaskConstants.TASK_MQTT_TOPIC)) {
+            LOGGER.info("task profile needs topic");
+            return false;
+        }
+        if (!profile.hasKey(TaskConstants.TASK_MQTT_SERVER_URI)) {
+            LOGGER.info("task profile needs serverUri");
+            return false;
+        }
+        if (!profile.hasKey(TaskConstants.TASK_MQTT_USERNAME)) {
+            LOGGER.info("task profile needs username");
+            return false;
+        }
+        if (!profile.hasKey(TaskConstants.TASK_MQTT_PASSWORD)) {
+            LOGGER.info("task profile needs password");
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    protected int getInstanceLimit() {
+        return DEFAULT_INSTANCE_LIMIT;

Review Comment:
   please make instanceLimit configurable



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "justinwwhuang (via GitHub)" <gi...@apache.org>.
justinwwhuang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1696164512


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/MqttSource.java:
##########
@@ -54,60 +50,76 @@ private List<Reader> splitSqlJob(String topics, String instanceId) {
         if (StringUtils.isEmpty(topics)) {
             return null;
         }
-        final List<Reader> result = new ArrayList<>();
+        final List<Reader> readers = new ArrayList<>();
         String[] topicList = topics.split(CommonConstants.COMMA);
-        if (Objects.nonNull(topicList)) {
-            Arrays.stream(topicList).forEach(topic -> {
-                MqttReader mqttReader = new MqttReader(topic);
-                mqttReader.setReadSource(instanceId);
-                result.add(mqttReader);
-            });
-        }
-        return result;
+        Arrays.stream(topicList).forEach(topic -> {
+            MqttReader mqttReader = new MqttReader(topic);
+            mqttReader.setReadSource(instanceId);
+            readers.add(mqttReader);
+        });
+        return readers;
     }
 
     @Override
     public List<Reader> split(TaskProfile conf) {
-        String topics = conf.get(JOB_MQTTJOB_TOPICS, StringUtils.EMPTY);
-        List<Reader> readerList = null;
-        if (StringUtils.isNotEmpty(topics)) {
-        }
-        if (CollectionUtils.isNotEmpty(readerList)) {
-            sourceMetric.sourceSuccessCount.incrementAndGet();
-        } else {
-            sourceMetric.sourceFailCount.incrementAndGet();
-        }
-        return readerList;
+        LOGGER.info("start to split mqtt source, conf:{}", conf);
+        String topics = conf.get(TaskConstants.TASK_MQTT_TOPIC, StringUtils.EMPTY);
+        return splitSqlJob(topics, instanceId);
     }
 
     @Override
     protected String getThreadName() {
-        return null;
+        return "mqtt-source-" + taskId + "-" + instanceId;
     }
 
     @Override
     protected void initSource(InstanceProfile profile) {
-
+        try {
+            LOGGER.info("MqttSource init: {}", profile.toJsonStr());
+            topic = profile.get(TaskConstants.TASK_MQTT_TOPIC);
+            mqttReader = new MqttReader(topic);
+            mqttReader.init(profile);
+        } catch (Exception e) {
+            stopRunning();

Review Comment:
   Suggest that the exception handling here be consistent with other sources



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang commented on PR #10727:
URL: https://github.com/apache/inlong/pull/10727#issuecomment-2255864356

   > I have fixed these issues, please review again, thank you!
   
   please click `resolve` if you fix the comment, thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "justinwwhuang (via GitHub)" <gi...@apache.org>.
justinwwhuang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1696177032


##########
inlong-agent/agent-plugins/src/test/java/org/apache/inlong/agent/plugin/sources/TestMqttReader.java:
##########
@@ -106,13 +107,13 @@ public void setUp() throws Exception {
 
         when(jobProfile.get(eq(CommonConstants.PROXY_INLONG_GROUP_ID), anyString())).thenReturn(groupId);

Review Comment:
   Sorry, it is recommended to put the reader's logic in the source. The current reader needs to be deleted after it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694563205


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/task/MqttTask.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.agent.plugin.task;
+
+import org.apache.inlong.agent.conf.InstanceProfile;
+import org.apache.inlong.agent.conf.TaskProfile;
+import org.apache.inlong.agent.constant.CycleUnitType;
+import org.apache.inlong.agent.constant.TaskConstants;
+import org.apache.inlong.agent.utils.AgentUtils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
+
+public class MqttTask extends AbstractTask {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(MqttTask.class);
+
+    private String topic;
+
+    private boolean isAdded = false;

Review Comment:
   please use AtiomicBoolean to ensure thread safety



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "qy-liuhuo (via GitHub)" <gi...@apache.org>.
qy-liuhuo commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1696284375


##########
inlong-agent/agent-plugins/src/test/java/org/apache/inlong/agent/plugin/sources/TestMqttReader.java:
##########
@@ -106,13 +107,13 @@ public void setUp() throws Exception {
 
         when(jobProfile.get(eq(CommonConstants.PROXY_INLONG_GROUP_ID), anyString())).thenReturn(groupId);

Review Comment:
   sorry, do you mean write the MqttReader's code in the mqttsource file? But how to handle the split function, just rerturn null as other sources?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694558122


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/MqttSource.java:
##########
@@ -20,63 +20,57 @@
 import org.apache.inlong.agent.conf.InstanceProfile;
 import org.apache.inlong.agent.conf.TaskProfile;
 import org.apache.inlong.agent.constant.CommonConstants;
+import org.apache.inlong.agent.constant.TaskConstants;
+import org.apache.inlong.agent.except.FileException;
 import org.apache.inlong.agent.plugin.Message;
 import org.apache.inlong.agent.plugin.file.Reader;
 import org.apache.inlong.agent.plugin.sources.file.AbstractSource;
 import org.apache.inlong.agent.plugin.sources.reader.MqttReader;
 
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Objects;
 
 public class MqttSource extends AbstractSource {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(MqttSource.class);
 
-    private static final String JOB_MQTTJOB_PARAM_PREFIX = "job.mqttJob.";
+    private MqttReader mqttReader;
 
-    private static final String JOB_MQTTJOB_SERVERURI = "";
-
-    private static final String JOB_MQTTJOB_CLIENTID = "";
-
-    public static final String JOB_MQTTJOB_TOPICS = "job.mqttJob.topic";
+    private String topic;
 
     public MqttSource() {
+        LOGGER.debug("MqttSource init!!!!!!!!!!!");
     }
 
     private List<Reader> splitSqlJob(String topics, String instanceId) {
         if (StringUtils.isEmpty(topics)) {
             return null;
         }
-        final List<Reader> result = new ArrayList<>();
+        final List<Reader> readers = new ArrayList<>();
         String[] topicList = topics.split(CommonConstants.COMMA);
-        if (Objects.nonNull(topicList)) {
-            Arrays.stream(topicList).forEach(topic -> {
-                MqttReader mqttReader = new MqttReader(topic);
-                mqttReader.setReadSource(instanceId);
-                result.add(mqttReader);
-            });
-        }
-        return result;
+        Arrays.stream(topicList).forEach(topic -> {
+            MqttReader mqttReader = new MqttReader(topic);
+            mqttReader.setReadSource(instanceId);
+            readers.add(mqttReader);
+        });
+        return readers;
     }
 
     @Override
     public List<Reader> split(TaskProfile conf) {
-        String topics = conf.get(JOB_MQTTJOB_TOPICS, StringUtils.EMPTY);
-        List<Reader> readerList = null;
-        if (StringUtils.isNotEmpty(topics)) {
-        }
-        if (CollectionUtils.isNotEmpty(readerList)) {
-            sourceMetric.sourceSuccessCount.incrementAndGet();
-        } else {
-            sourceMetric.sourceFailCount.incrementAndGet();
-        }
+        LOGGER.info("splited!!!");
+        String topics = conf.get(TaskConstants.TASK_MQTT_TOPIC, StringUtils.EMPTY);
+        List<Reader> readerList = splitSqlJob(topics, instanceId);
+        // if (CollectionUtils.isNotEmpty(readerList)) {

Review Comment:
   please remove these code



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "qy-liuhuo (via GitHub)" <gi...@apache.org>.
qy-liuhuo commented on PR #10727:
URL: https://github.com/apache/inlong/pull/10727#issuecomment-2255874275

   > > I have fixed these issues, please review again, thank you!
   > 
   > please click `resolve` if you fix the comment, thanks.
   
   ok,thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "qy-liuhuo (via GitHub)" <gi...@apache.org>.
qy-liuhuo commented on PR #10727:
URL: https://github.com/apache/inlong/pull/10727#issuecomment-2255089171

   I have fixed these issues, please review again, thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "justinwwhuang (via GitHub)" <gi...@apache.org>.
justinwwhuang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1696365462


##########
inlong-agent/agent-plugins/src/test/java/org/apache/inlong/agent/plugin/sources/TestMqttReader.java:
##########
@@ -106,13 +107,13 @@ public void setUp() throws Exception {
 
         when(jobProfile.get(eq(CommonConstants.PROXY_INLONG_GROUP_ID), anyString())).thenReturn(groupId);

Review Comment:
   > sorry, do you mean write the MqttReader's code in the mqttsource file? But how to handle the split function, just rerturn null as other sources?
   
   yes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "aloyszhang (via GitHub)" <gi...@apache.org>.
aloyszhang merged PR #10727:
URL: https://github.com/apache/inlong/pull/10727


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "qy-liuhuo (via GitHub)" <gi...@apache.org>.
qy-liuhuo commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1696366855


##########
inlong-agent/agent-plugins/src/test/java/org/apache/inlong/agent/plugin/sources/TestMqttReader.java:
##########
@@ -106,13 +107,13 @@ public void setUp() throws Exception {
 
         when(jobProfile.get(eq(CommonConstants.PROXY_INLONG_GROUP_ID), anyString())).thenReturn(groupId);

Review Comment:
   ok, i will fix it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "aloyszhang (via GitHub)" <gi...@apache.org>.
aloyszhang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694632543


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/reader/MqttReader.java:
##########
@@ -147,6 +137,7 @@ public void deliveryComplete(IMqttDeliveryToken token) {
                     }
                 });
                 client.connect(options);
+                Thread.sleep(1000);

Review Comment:
   what this `sleep` for?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694559483


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/MqttSource.java:
##########
@@ -20,63 +20,57 @@
 import org.apache.inlong.agent.conf.InstanceProfile;
 import org.apache.inlong.agent.conf.TaskProfile;
 import org.apache.inlong.agent.constant.CommonConstants;
+import org.apache.inlong.agent.constant.TaskConstants;
+import org.apache.inlong.agent.except.FileException;
 import org.apache.inlong.agent.plugin.Message;
 import org.apache.inlong.agent.plugin.file.Reader;
 import org.apache.inlong.agent.plugin.sources.file.AbstractSource;
 import org.apache.inlong.agent.plugin.sources.reader.MqttReader;
 
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Objects;
 
 public class MqttSource extends AbstractSource {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(MqttSource.class);
 
-    private static final String JOB_MQTTJOB_PARAM_PREFIX = "job.mqttJob.";
+    private MqttReader mqttReader;
 
-    private static final String JOB_MQTTJOB_SERVERURI = "";
-
-    private static final String JOB_MQTTJOB_CLIENTID = "";
-
-    public static final String JOB_MQTTJOB_TOPICS = "job.mqttJob.topic";
+    private String topic;
 
     public MqttSource() {
+        LOGGER.debug("MqttSource init!!!!!!!!!!!");
     }
 
     private List<Reader> splitSqlJob(String topics, String instanceId) {
         if (StringUtils.isEmpty(topics)) {
             return null;
         }
-        final List<Reader> result = new ArrayList<>();
+        final List<Reader> readers = new ArrayList<>();
         String[] topicList = topics.split(CommonConstants.COMMA);
-        if (Objects.nonNull(topicList)) {
-            Arrays.stream(topicList).forEach(topic -> {
-                MqttReader mqttReader = new MqttReader(topic);
-                mqttReader.setReadSource(instanceId);
-                result.add(mqttReader);
-            });
-        }
-        return result;
+        Arrays.stream(topicList).forEach(topic -> {
+            MqttReader mqttReader = new MqttReader(topic);
+            mqttReader.setReadSource(instanceId);
+            readers.add(mqttReader);
+        });
+        return readers;
     }
 
     @Override
     public List<Reader> split(TaskProfile conf) {
-        String topics = conf.get(JOB_MQTTJOB_TOPICS, StringUtils.EMPTY);
-        List<Reader> readerList = null;
-        if (StringUtils.isNotEmpty(topics)) {
-        }
-        if (CollectionUtils.isNotEmpty(readerList)) {
-            sourceMetric.sourceSuccessCount.incrementAndGet();
-        } else {
-            sourceMetric.sourceFailCount.incrementAndGet();
-        }
+        LOGGER.info("splited!!!");

Review Comment:
   please log more specific information like "start to split mqtt source, conf={}"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694560301


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/MqttSource.java:
##########
@@ -87,27 +81,51 @@ protected String getThreadName() {
 
     @Override
     protected void initSource(InstanceProfile profile) {
-
+        try {
+            LOGGER.info("MqttSource init: {}", profile.toJsonStr());
+            topic = profile.get(TaskConstants.TASK_MQTT_TOPIC);
+            mqttReader = new MqttReader(topic);
+            mqttReader.init(profile);
+        } catch (Exception e) {
+            stopRunning();

Review Comment:
   please log the exception why stop initing source



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "vernedeng (via GitHub)" <gi...@apache.org>.
vernedeng commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694561750


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/reader/MqttReader.java:
##########
@@ -246,4 +237,26 @@ public void destroy() {
             }
         }
     }
+
+    public static void main(String[] args) throws InterruptedException {

Review Comment:
   please remove test code



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "aloyszhang (via GitHub)" <gi...@apache.org>.
aloyszhang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694628418


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/MqttSource.java:
##########
@@ -20,63 +20,57 @@
 import org.apache.inlong.agent.conf.InstanceProfile;
 import org.apache.inlong.agent.conf.TaskProfile;
 import org.apache.inlong.agent.constant.CommonConstants;
+import org.apache.inlong.agent.constant.TaskConstants;
+import org.apache.inlong.agent.except.FileException;
 import org.apache.inlong.agent.plugin.Message;
 import org.apache.inlong.agent.plugin.file.Reader;
 import org.apache.inlong.agent.plugin.sources.file.AbstractSource;
 import org.apache.inlong.agent.plugin.sources.reader.MqttReader;
 
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Objects;
 
 public class MqttSource extends AbstractSource {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(MqttSource.class);
 
-    private static final String JOB_MQTTJOB_PARAM_PREFIX = "job.mqttJob.";
+    private MqttReader mqttReader;
 
-    private static final String JOB_MQTTJOB_SERVERURI = "";
-
-    private static final String JOB_MQTTJOB_CLIENTID = "";
-
-    public static final String JOB_MQTTJOB_TOPICS = "job.mqttJob.topic";
+    private String topic;
 
     public MqttSource() {
+        LOGGER.debug("MqttSource init!!!!!!!!!!!");
     }
 
     private List<Reader> splitSqlJob(String topics, String instanceId) {
         if (StringUtils.isEmpty(topics)) {
             return null;
         }
-        final List<Reader> result = new ArrayList<>();
+        final List<Reader> readers = new ArrayList<>();
         String[] topicList = topics.split(CommonConstants.COMMA);
-        if (Objects.nonNull(topicList)) {
-            Arrays.stream(topicList).forEach(topic -> {
-                MqttReader mqttReader = new MqttReader(topic);
-                mqttReader.setReadSource(instanceId);
-                result.add(mqttReader);
-            });
-        }
-        return result;
+        Arrays.stream(topicList).forEach(topic -> {
+            MqttReader mqttReader = new MqttReader(topic);
+            mqttReader.setReadSource(instanceId);
+            readers.add(mqttReader);
+        });
+        return readers;
     }
 
     @Override
     public List<Reader> split(TaskProfile conf) {
-        String topics = conf.get(JOB_MQTTJOB_TOPICS, StringUtils.EMPTY);
-        List<Reader> readerList = null;
-        if (StringUtils.isNotEmpty(topics)) {
-        }
-        if (CollectionUtils.isNotEmpty(readerList)) {
-            sourceMetric.sourceSuccessCount.incrementAndGet();
-        } else {
-            sourceMetric.sourceFailCount.incrementAndGet();
-        }
+        LOGGER.info("splited!!!");
+        String topics = conf.get(TaskConstants.TASK_MQTT_TOPIC, StringUtils.EMPTY);
+        List<Reader> readerList = splitSqlJob(topics, instanceId);

Review Comment:
   Local variable 'readerList' is redundant, just return `splitSqlJob(topics, instanceId)` is ok.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [INLONG-10286][Agent] Update the MQTT Source [inlong]

Posted by "aloyszhang (via GitHub)" <gi...@apache.org>.
aloyszhang commented on code in PR #10727:
URL: https://github.com/apache/inlong/pull/10727#discussion_r1694630633


##########
inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sources/reader/MqttReader.java:
##########
@@ -88,22 +77,22 @@ public MqttReader(String topic) {
      * @param jobConf
      */
     private void setGlobalParamsValue(InstanceProfile jobConf) {
-        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(JOB_MQTT_QUEUE_SIZE, 1000));
+        mqttMessagesQueue = new LinkedBlockingQueue<>(jobConf.getInt(TaskConstants.TASK_MQTT_QUEUE_SIZE, 1000));
         instanceId = jobConf.getInstanceId();
-        userName = jobConf.get(JOB_MQTT_USERNAME);
-        password = jobConf.get(JOB_MQTT_PASSWORD);
-        serverURI = jobConf.get(JOB_MQTT_SERVER_URI);
-        topic = jobConf.get(JOB_MQTT_TOPIC);
-        clientId = jobConf.get(JOB_MQTT_CLIENT_ID_PREFIX, "mqtt_client") + "_" + UUID.randomUUID();
-        cleanSession = jobConf.getBoolean(JOB_MQTT_CLEAN_SESSION, false);
-        automaticReconnect = jobConf.getBoolean(JOB_MQTT_AUTOMATIC_RECONNECT, true);
-        qos = jobConf.getInt(JOB_MQTT_QOS, 1);
-        mqttVersion = jobConf.getInt(JOB_MQTT_VERSION, MqttConnectOptions.MQTT_VERSION_DEFAULT);
-
+        userName = jobConf.get(TaskConstants.TASK_MQTT_USERNAME);
+        password = jobConf.get(TaskConstants.TASK_MQTT_PASSWORD);
+        serverURI = jobConf.get(TaskConstants.TASK_MQTT_SERVER_URI);
+
+        // topic = jobConf.get(TaskConstants.TASK_MQTT_TOPIC);

Review Comment:
   remove this line if not used.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org