You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by "haifxu (via GitHub)" <gi...@apache.org> on 2023/03/20 02:39:54 UTC
[GitHub] [inlong] haifxu opened a new pull request, #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
haifxu opened a new pull request, #7647:
URL: https://github.com/apache/inlong/pull/7647
### Prepare a Pull Request
- Fixes #7646
### Motivation
When the MQ configuration has not been registered in the manager, Audit obtains the MQ configuration and causes NPE.
企业微信截图_16792136908776
### Modifications
If the MQ configuration is not obtained, wait for 5 seconds and retry.
--
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
[GitHub] [inlong] healchow commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141587572
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
Review Comment:
Do we need to set a maximum number of retries? If it exceeds, it will fail, otherwise, the program will loop infinitely.
--
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
[GitHub] [inlong] dockerzhang commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141589147
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
Review Comment:
I think there is no need to set a maximum number of retries, because MQ is a necessary service for audit and other InLong components, we don't know when the users would configure the MQ cluster, it's better to keep waiting.
--
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
[GitHub] [inlong] healchow commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "healchow (via GitHub)" <gi...@apache.org>.
healchow commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141601112
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
Review Comment:
Ok, let's change it like this and keep watching.
--
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
[GitHub] [inlong] dockerzhang commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141587622
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
+ mqConfig = getMQConfig(host, clusterTag);
+ if (ObjectUtils.isNotEmpty(mqConfig)) {
+ return mqConfig;
+ }
+ LOG.info("MQ config may not be registered yet, wait for 5s and try again");
+ Thread.sleep(INTERVAL_MS);
Review Comment:
it's not a common configuration, I think there is no need to add to configuration files.
--
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
[GitHub] [inlong] dockerzhang merged pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang merged PR #7647:
URL: https://github.com/apache/inlong/pull/7647
--
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
[GitHub] [inlong] dockerzhang commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "dockerzhang (via GitHub)" <gi...@apache.org>.
dockerzhang commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141589147
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
Review Comment:
there is no need to set a maximum number of retries, because MQ is a necessary service for audit and other InLong components, we don't know when the users would configure the MQ cluster, it's better to keep waiting.
--
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
[GitHub] [inlong] doleyzi commented on a diff in pull request #7647: [INLONG-7646][Audit] Fix NPE when mq configuration is not registered
Posted by "doleyzi (via GitHub)" <gi...@apache.org>.
doleyzi commented on code in PR #7647:
URL: https://github.com/apache/inlong/pull/7647#discussion_r1141572322
##########
inlong-audit/audit-store/src/main/java/org/apache/inlong/audit/service/AuditMsgConsumerServer.java:
##########
@@ -133,19 +135,23 @@ private List<InsertData> getInsertServiceList() {
private List<MQInfo> getClusterFromManager() {
Properties properties = new Properties();
+ List<MQInfo> mqConfig;
try (InputStream inputStream = getClass().getClassLoader().getResourceAsStream(DEFAULT_CONFIG_PROPERTIES)) {
properties.load(inputStream);
String managerHosts = properties.getProperty("manager.hosts");
String clusterTag = properties.getProperty("proxy.cluster.tag");
String[] hostList = StringUtils.split(managerHosts, ",");
for (String host : hostList) {
- List<MQInfo> mqConfig = getMQConfig(host, clusterTag);
- if (ObjectUtils.isNotEmpty(mqConfig)) {
- LOG.info("return mqConfig");
- return mqConfig;
+ while (true) {
+ mqConfig = getMQConfig(host, clusterTag);
+ if (ObjectUtils.isNotEmpty(mqConfig)) {
+ return mqConfig;
+ }
+ LOG.info("MQ config may not be registered yet, wait for 5s and try again");
+ Thread.sleep(INTERVAL_MS);
Review Comment:
It is recommended to use configuration files for configuration
--
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