You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2024/12/24 10:06:57 UTC
[PR] KAFKA-18274: Failed to restart controller in testing due to closed socket channel [1/2] [kafka]
peterxcli opened a new pull request, #18310:
URL: https://github.com/apache/kafka/pull/18310
We should rebuild the metrics of `SharedServer` when restart the controller in test to prevent NPE.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-18274: Failed to restart controller in testing due to closed socket channel [1/2] [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #18310:
URL: https://github.com/apache/kafka/pull/18310
--
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-18274: Failed to restart controller in testing due to closed socket channel [1/2] [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18310:
URL: https://github.com/apache/kafka/pull/18310#discussion_r1896715858
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -155,6 +156,11 @@ class ControllerServer(
() => featuresPublisher.features()
)
+ // metrics will be set to null when closing a controller, so we should recreate it for testing
+ if (sharedServer.metrics == null){
Review Comment:
Should we do similar fix to BrokerServer? It uses the metrics as well.
--
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-18274: Failed to restart controller in testing due to closed socket channel [1/2] [kafka]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #18310:
URL: https://github.com/apache/kafka/pull/18310#discussion_r1897059235
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -155,6 +156,11 @@ class ControllerServer(
() => featuresPublisher.features()
)
+ // metrics will be set to null when closing a controller, so we should recreate it for testing
+ if (sharedServer.metrics == null){
Review Comment:
Hi, @chia7712 Thanks for your review.
Regarding the `BrokerServer`, it calls `sharedServer.startForBroker()` at the beginning of the `startup` method, which also [checks if the metrics are null](https://github.com/apache/kafka/blob/ccbb73111c0d4bf1c999e8dd5f9f111393508046/core/src/main/scala/kafka/server/SharedServer.scala#L266-L270)
However, we cannot apply the same approach to the `ControllerServer` because `sharedServer.startForController()` in it depends on `listenerInfo`. Therefore, this change is necessary.
Here is the relevant reference:
- [BrokerServer startup method](https://github.com/apache/kafka/blob/ebb3202e01d9481d7e711cd25e4e564863b91e1e/core/src/main/scala/kafka/server/BrokerServer.scala#L187-L616)
- [listenerInfo dependence in ControllerServer](https://github.com/apache/kafka/blob/7408623bb65c182404d5ce70b50c480c41f1abc2/core/src/main/scala/kafka/server/ControllerServer.scala#L166-L190)
--
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-18274: Failed to restart controller in testing due to closed socket channel [1/2] [kafka]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #18310:
URL: https://github.com/apache/kafka/pull/18310#issuecomment-2561564685
Unrelated tests failed🥲:
Java23:
SaslSslAdminIntegrationTest | "testCreateTopicsResponseMetadataAndConfig(String).quorum=kraft" | org.opentest4j.AssertionFailedError: Expected java.util.concurrent.ExecutionException to be thrown, but nothing was thrown. | 7.02s
-- | -- | -- | --
PlaintextAdminIntegrationTest | testConsumerGroupsDeprecatedConsumerGroupState(String, String).quorum=kraft.groupProtocol=consumer | org.opentest4j.AssertionFailedError: Expected the offset for partition 0 to eventually become 1. | 21.58s
PlaintextAdminIntegrationTest | testConsumerGroups(String, String).quorum=kraft.groupProtocol=consumer | org.opentest4j.AssertionFailedError: Expected the offset for partition 0 to eventually become 1. | 22.18s
Java17:
AbstractCoordinatorTest | testWakeupAfterSyncGroupReceivedExternalCompletion() | org.opentest4j.AssertionFailedError: Should have woken up from ensureActiveGroup() ==> Expected org.apache.kafka.common.errors.WakeupException to be thrown, but nothing was thrown. | 0.48s
-- | -- | -- | --
--
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