You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "fanyang (via GitHub)" <gi...@apache.org> on 2025/09/20 10:25:21 UTC
[PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
fanyang opened a new pull request, #20564:
URL: https://github.com/apache/kafka/pull/20564
The PR fixed an NPE when connectorStatusMetrics unregistering task. The issue will happen if a connector has more than 1 task on one worker and these tasks fail to start. For example, if the DNS cant resolve producer URL, tasks will fail to build. If these tasks failing to start, ConnectorStatusMetricsGroup will unregister them. After first task unregistered , connectorStatusMetrics removed connector. When second task calles recordTaskRemoved(), connectorStatusMetrics is empty, so connectorStatusMetrics.get(connectorTaskId.connector()).close(); will throw NPE.
Like in method recordTaskAdded(), this fix checkes connectorStatusMetrics at the beginning of method recordTaskRemoved().
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "Pangstar (via GitHub)" <gi...@apache.org>.
Pangstar commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4030617522
This has also affect us in production and we rolled back but would be good to get this in ASAP please.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "fanyang (via GitHub)" <gi...@apache.org>.
fanyang commented on code in PR #20564:
URL: https://github.com/apache/kafka/pull/20564#discussion_r2377466124
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java:
##########
@@ -898,6 +899,36 @@ public void testConnectorStatusMetricsGroup_taskStatusCounter(boolean enableTopi
verifyKafkaClusterId();
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testConnectorStatusMetricsGroup_tasksFailedToStart(boolean enableTopicCreation) {
+ setup(enableTopicCreation);
+ ConcurrentMap<ConnectorTaskId, WorkerTask<?, ?>> tasks = new ConcurrentHashMap<>();
Review Comment:
Thanks. It's hard to create an immutable ConcurrentHashMap. I directly pass an empty ConcurrentHashMap to metrics group.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4068730315
@fanyang Thanks for your contribution! I've added you as a co-author on the commit.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-3717220524
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "beregon87 (via GitHub)" <gi...@apache.org>.
beregon87 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-3759723119
Can someone please review this?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4032119583
@fanyang would you mind merging the trunk to run QA again?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "fanyang (via GitHub)" <gi...@apache.org>.
fanyang commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-3316508425
Added UT. Thanks @zheguang
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "zheguang (via GitHub)" <gi...@apache.org>.
zheguang commented on code in PR #20564:
URL: https://github.com/apache/kafka/pull/20564#discussion_r2375570585
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java:
##########
@@ -898,6 +899,36 @@ public void testConnectorStatusMetricsGroup_taskStatusCounter(boolean enableTopi
verifyKafkaClusterId();
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testConnectorStatusMetricsGroup_tasksFailedToStart(boolean enableTopicCreation) {
+ setup(enableTopicCreation);
+ ConcurrentMap<ConnectorTaskId, WorkerTask<?, ?>> tasks = new ConcurrentHashMap<>();
Review Comment:
Minor: to help with reading the test, I'd name this as `emptyTasks` instantiated with immutable empty map.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4043239993
I’ve submitted a new patch to help this PR get merged more quickly. https://github.com/apache/kafka/pull/21717
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 closed pull request #20564: KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa…
URL: https://github.com/apache/kafka/pull/20564
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "zheguang (via GitHub)" <gi...@apache.org>.
zheguang commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4027332065
:thinking: from recent merges to `connect`, I'm guessing/hoping @frankvicky could help with a quick approving review? (Sorry if I spammed...)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "beregon87 (via GitHub)" <gi...@apache.org>.
beregon87 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4066752002
@chia7712 can you please take over this PR?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4066781241
@beregon87 we will handle this issue in #21717 instead
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4032342225
@beregon87 @pdavidson100 @zheguang @Pangstar Sorry for the late review. We will take over this PR if @fanyang does not have enough bandwidth
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "zheguang (via GitHub)" <gi...@apache.org>.
zheguang commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-3316445907
Thanks for catching the bug, @fanyang. Probably easy to nail the scenario in a a small test case to `WorkerTest`, similar to `testConnectorStatusMetricsGroup_taskStatusCounter` [here](https://github.com/apache/kafka/blob/01fccd35138b407d87828ceb557d33c4b5450cac/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTest.java#L872), such as
```java
assertDoesNotThrow(() -> metricsGroup.recordTaskRemoved(new ConnectorTaskId('nonadded-task')), "should not throw NPE").
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20564:
URL: https://github.com/apache/kafka/pull/20564#discussion_r2912608692
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java:
##########
@@ -2394,6 +2394,10 @@ protected synchronized void recordTaskAdded(ConnectorTaskId connectorTaskId) {
}
protected synchronized void recordTaskRemoved(ConnectorTaskId connectorTaskId) {
Review Comment:
Could you consider using the following style:
```java
protected synchronized void recordTaskRemoved(ConnectorTaskId connectorTaskId) {
// Unregister connector task count metric if we remove the last task of the connector
if (tasks.keySet().stream().noneMatch(id -> id.connector().equals(connectorTaskId.connector()))) {
var metricGroup = connectorStatusMetrics.remove(connectorTaskId.connector());
if (metricGroup != null) metricGroup.close();
}
}
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19697: Fix unregistering connectorStatusMetrics NPE if tasks fa… [kafka]
Posted by "pdavidson100 (via GitHub)" <gi...@apache.org>.
pdavidson100 commented on PR #20564:
URL: https://github.com/apache/kafka/pull/20564#issuecomment-4014387730
This bug recently caused a problem for us in production, so would be great to get this fix merged. @zheguang looks like you reviewed but there's a problem with the CI build? How do we get this unstuck?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org