You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "showuon (via GitHub)" <gi...@apache.org> on 2023/04/27 09:52:32 UTC
[GitHub] [kafka] showuon opened a new pull request, #13647: MINOR: fix KRaftClusterTest
showuon opened a new pull request, #13647:
URL: https://github.com/apache/kafka/pull/13647
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### 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
[GitHub] [kafka] ijuma commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1527834450
@mumrah the original PR had the same failures, how come we merged it?
--
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
[GitHub] [kafka] cmccabe commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1526021292
Yes, the fault handler should be invoked only for non-`ApiException` classes
(Technically `ApiException` itself is a `RuntimException` ... a questionable decision but there's no changing it now)
--
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
[GitHub] [kafka] showuon closed pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon closed pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
URL: https://github.com/apache/kafka/pull/13647
--
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
[GitHub] [kafka] showuon commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1525592758
@mumrah , please take a look. Thanks.
--
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
[GitHub] [kafka] showuon commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1526879080
Fixing the issue in https://github.com/apache/kafka/pull/13651
--
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
[GitHub] [kafka] mumrah commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1528072990
The test failure was introduced by a commit fairly late in #13407. I did briefly investigate it, but couldn't reproduce it locally, so I figured it was existing flakiness. Basically, it's just my fault for not looking more closely at the test failures.
--
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
[GitHub] [kafka] mumrah commented on pull request #13647: MINOR: fix KRaftClusterTest and KRaft integration test failure
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #13647:
URL: https://github.com/apache/kafka/pull/13647#issuecomment-1525728492
Ah, looks like I introduced this problem. You're correct that we shouldn't call the fault handler here for expected errors like a controller failover. I added this logic to catch other errors inside the activation event regarding migration state. For example, if an established KRaft cluster is restarted with migrations enabled, we should terminate the controller with an error. Since we actually need to read some state from the metadata log to determine this, we can't just do a simple config validation as we start ControllerServer.
Can we keep the exception handler, but only call the fault handler for RuntimeExceptions?
--
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