Skip to content

Conversation

@fanyang
Copy link
Contributor

@fanyang fanyang commented Sep 20, 2025

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().

@github-actions github-actions bot added triage PRs from the community connect small Small PRs labels Sep 20, 2025
@fanyang fanyang marked this pull request as ready for review September 20, 2025 10:26
@zheguang
Copy link
Contributor

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, such as

assertDoesNotThrow(() -> metricsGroup.recordTaskRemoved(new ConnectorTaskId('nonadded-task')), "should not throw NPE").

@fanyang
Copy link
Contributor Author

fanyang commented Sep 22, 2025

Added UT. Thanks @zheguang

@ValueSource(booleans = {true, false})
public void testConnectorStatusMetricsGroup_tasksFailedToStart(boolean enableTopicCreation) {
setup(enableTopicCreation);
ConcurrentMap<ConnectorTaskId, WorkerTask<?, ?>> tasks = new ConcurrentHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: to help with reading the test, I'd name this as emptyTasks instantiated with immutable empty map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. It's hard to create an immutable ConcurrentHashMap. I directly pass an empty ConcurrentHashMap to metrics group.

@github-actions github-actions bot removed the triage PRs from the community label Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants