You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "edoardocomar (via GitHub)" <gi...@apache.org> on 2025/02/07 16:10:41 UTC
[PR] KAFKA-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
edoardocomar opened a new pull request, #18833:
URL: https://github.com/apache/kafka/pull/18833
Add checks for null in shutdown as BrokerLifecycleManager is not instantiaited if LogManager constructor throws an Exception
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2650435130
I will instead cherry-pick on 3.9 where the NPE still occurs.
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2650371001
I would like to cherry pick this in 4.0.x but it may be too late for 4.0.0 and I perhaps should wait for 4.0.1 ?
I'll ask on the mailing list
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949925944
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
I also think this seems a bit odd. Having gone to all of the trouble to null-check references to lifecycleManager, I would have expected something more like:
```
if (logManager != null)
CoreUtils.swallow(logManager.shutdown(if (lifecycleManager != null) lifecycleManager.brokerEpoch else -1, 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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949584155
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
Yeah, I understand that. But there could be another case where `lifecycleManager` construction fails, in that case ideally we should close `logManager`, if instantiated, correct?
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2643591288
commit also cherry picks cleanly on 4.0
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949587802
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
Will it be safe to pass `-1` as broker epoch to `logManager.shutdown` as the broker is not compeletely instantiated?
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949986081
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
thanks, pushed a change
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949535276
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
Does that mean if there is an error instantiating `lifecycleManager` then we will also not shutdown `logManager`?
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar merged PR #18833:
URL: https://github.com/apache/kafka/pull/18833
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2648500129
thank you @gaurav-narula - I am retrying the failed flaky tests
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2648839225
CI rerun - only flaky failures 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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2648750784
```
./gradlew core:test --tests DynamicBrokerReconfigurationTest
./gradlew storage:test --tests DeleteSegmentsByRetentionTimeTest
```
also pass for me locally with java 23
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949587802
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
Will it be safe to pass `-1` as broker epoch to `logManager.shutdown` as the broker is not compeletely instantiated, in case `lifecycleManager` 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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2649382280
thanks @AndrewJSchofield and @apoorvmittal10
I also removed a few braces to match the style of nearby if statements
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2650570476
cherry picked on 4.0.0 too
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on PR #18833:
URL: https://github.com/apache/kafka/pull/18833#issuecomment-2648554344
```
./gradlew client:test --tests AbstractCoordinatorTest
./gradlew core:test --tests ReplicationQuotasTest
./gradlew core:test --tests RequestQuotaTest
```
all pass for me with Java 17
--
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-18758: NullPointerException in shutdown following InvalidConfigurationException [kafka]
Posted by "edoardocomar (via GitHub)" <gi...@apache.org>.
edoardocomar commented on code in PR #18833:
URL: https://github.com/apache/kafka/pull/18833#discussion_r1949573689
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -803,7 +807,7 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
+ if (logManager != null && lifecycleManager != null)
Review Comment:
Thanks @apoorvmittal10
the body of the if
```
if (logManager != null && lifecycleManager != null)
CoreUtils.swallow(logManager.shutdown(lifecycleManager.brokerEpoch), this)
```
so if `lifecycleManager` was null but `logManager` was not, there still would be an NPE.
having said that,
the reason I encountered the NPE was because logManager creation failed, so logManager was null and the body of the if would have been skipped anyway.
--
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