You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/03/03 12:52:57 UTC
[GitHub] [incubator-inlong] EMsnap opened a new pull request #2878: [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
EMsnap opened a new pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878
### [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
Fixes #2877
### Motivation
Agent task position manager throws npe when send dataproxy ack success
### Modifications
Agent task position manager throws npe when send dataproxy ack success
when there is no source, the position should not be stored locally since we report snapshot to manager already### Verifying this change
- [ ] Make sure that the change passes the CI checks.
### Documentation
- Does this pull request introduce a new feature? (no)
--
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] [incubator-inlong] EMsnap closed pull request #2878: [INLONG-2877][Agent] Task position manager throws NPE when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
EMsnap closed pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878
--
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] [incubator-inlong] EMsnap commented on a change in pull request #2878: [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
EMsnap commented on a change in pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#discussion_r818727314
##########
File path: inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sinks/SenderManager.java
##########
@@ -206,7 +206,13 @@ public void onMessageAck(SendResult result) {
return;
}
metric.incSendSuccessNum(bodyList.size());
- taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ /**
+ * when there is no source, the position should not be stored locally
+ * since we report snapshot to manager already
+ */
+ if (sourcePath != null) {
+ taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ }
Review comment:
Actually only file task need position to be stored locally, so here source path with null value is normal.
We will remove this taskPositionManager in later version since we've updated the position in manager already.
--
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] [incubator-inlong] EMsnap commented on pull request #2878: [INLONG-2877][Agent] Task position manager throws NPE when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
EMsnap commented on pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#issuecomment-1058770915
Solved in https://github.com/apache/incubator-inlong/pull/2881#pullrequestreview-899743725
--
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] [incubator-inlong] aloyszhang commented on a change in pull request #2878: [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#discussion_r818636958
##########
File path: inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sinks/SenderManager.java
##########
@@ -206,7 +206,13 @@ public void onMessageAck(SendResult result) {
return;
}
metric.incSendSuccessNum(bodyList.size());
- taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ /**
+ * when there is no source, the position should not be stored locally
+ * since we report snapshot to manager already
+ */
+ if (sourcePath != null) {
+ taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ }
Review comment:
Is there any need to add some logs for troubleshooting when `sourcePath` is null?
--
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] [incubator-inlong] aloyszhang commented on a change in pull request #2878: [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#discussion_r818636958
##########
File path: inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sinks/SenderManager.java
##########
@@ -206,7 +206,13 @@ public void onMessageAck(SendResult result) {
return;
}
metric.incSendSuccessNum(bodyList.size());
- taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ /**
+ * when there is no source, the position should not be stored locally
+ * since we report snapshot to manager already
+ */
+ if (sourcePath != null) {
+ taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ }
Review comment:
Is there any need to add some logs for troubleshooting if `sourcePath` is null?
--
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] [incubator-inlong] healchow commented on a change in pull request #2878: [INLONG-2877][Agent]Agent task position manager throws npe when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
healchow commented on a change in pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#discussion_r818780756
##########
File path: inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sinks/SenderManager.java
##########
@@ -206,7 +206,13 @@ public void onMessageAck(SendResult result) {
return;
}
metric.incSendSuccessNum(bodyList.size());
- taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ /**
Review comment:
It's not necessary to use Javadoc in any method.
--
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] [incubator-inlong] EMsnap commented on a change in pull request #2878: [INLONG-2877][Agent] Task position manager throws NPE when send dataproxy ack success
Posted by GitBox <gi...@apache.org>.
EMsnap commented on a change in pull request #2878:
URL: https://github.com/apache/incubator-inlong/pull/2878#discussion_r819221921
##########
File path: inlong-agent/agent-plugins/src/main/java/org/apache/inlong/agent/plugin/sinks/SenderManager.java
##########
@@ -206,7 +206,13 @@ public void onMessageAck(SendResult result) {
return;
}
metric.incSendSuccessNum(bodyList.size());
- taskPositionManager.updateSinkPosition(jobId, sourcePath, bodyList.size());
+ /**
Review comment:
It's confusing if I don't add this comment
--
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