You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "erikwright (via GitHub)" <gi...@apache.org> on 2026/10/21 22:13:49 UTC

[PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

erikwright opened a new pull request, #5839:
URL: https://github.com/apache/polaris/pull/5839

`KafkaEventListenerTest` sometimes fails with `initializationError` before any test runs, because the Kafka container does not log that it is running within the wait strategy's 60 seconds (`ContainerLaunchException` at `LogMessageWaitStrategy`). It has failed this way on unrelated PRs, for example in [#5310's run](https://github.com/apache/polaris/actions/runs/32827840903/job/100064478924) and [#5312's run](https://github.com/apache/polaris/actions/runs/33609671316/job/100468595185) (see also the comments on #5222 and #5312). In a failure where I could see the container output, the image had pulled in 3 seconds and the container printed its launch script, then nothing more for the whole 60 seconds. That looks like a stuck start, not a slow one.

This change allows three start attempts, as the floci test containers already do. Testcontainers removes the container of a failed attempt and starts a new one. Only the container start is retried, not the tests.

I don't know why the broker sometimes hangs. If the cause is the runner, for example disk I/O while the parallel build runs, all three attempts could still fail, and a longer wait would be the next thing to try.

Testing:
- I ran `KafkaEventListenerTest` 5 times locally (Podman, `apache/kafka-native:4.3.2`): all passed. The hang did not happen locally, so the retry path itself was not exercised.
- `./gradlew format compileAll` and `./gradlew :polaris-extensions-events-kafka:check` pass.

AI disclosure: the diagnosis, the testing above, this change and this description were done with an AI coding agent.

## Checklist
- [x] 🛡️ Don't disclose security issues! (contact security@apache.org)
- [x] 🔗 Clearly explained why the changes are needed, or linked related issues: no issue filed
- [x] 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
- [x] 💡 Added comments for complex logic
- [ ] 🧾 Updated `CHANGELOG.md` (if needed): not needed, test-only change
- [ ] 📚 Updated documentation in `site/content/in-dev/unreleased` (if needed): not needed

--
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.

ayushtkn merged PR #5839:
URL: https://github.com/apache/polaris/pull/5839


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

Posted by "erikwright (via GitHub)" <gi...@apache.org>.

erikwright commented on code in PR #5839:
URL: https://github.com/apache/polaris/pull/5839#discussion_r4351175112


##########
extensions/events/kafka/src/test/java/org/apache/polaris/extensions/events/kafka/KafkaEventListenerTest.java:
##########
@@ -65,12 +65,17 @@
 @Testcontainers
 @ExtendWith(MockitoExtension.class)
 class KafkaEventListenerTest {
+  // apache/kafka-native images built with GraalVM < 25 segfault in getpwuid() on about 2% of
+  // starts (KAFKA-20314). Testcontainers only notices once the 60 second log wait times out, so
+  // allow more attempts; each starts a new container. Remove once the image is 4.4.0 or later.
+  // See https://github.com/apache/polaris/issues/5845.

Review Comment:
   Removed in the latest commit; the PR description links the issue. (Agent-drafted on my behalf, not reviewed.)



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

Posted by "erikwright (via GitHub)" <gi...@apache.org>.

erikwright commented on PR #5839:
URL: https://github.com/apache/polaris/pull/5839#issuecomment-6331301750

   @dimas-b sorry, one more look needed: the force-push reset your approval. The code change is the same (`withStartupAttempts(3)`); what changed is that I found the actual cause and rewrote the comment, commit message and description to reflect it.
   
   In the failing runs the container is not hanging: `apache/kafka-native` segfaults in `getpwuid()` about 10 ms after launch ([KAFKA-20314](https://issues.apache.org/jira/browse/KAFKA-20314), a GraalVM 21 bug fixed in GraalVM 25), and Testcontainers only reports it once the 60 second log wait times out. Details and the crash dumps from our CI test reports are in #5845. The retry is still the right mitigation until `apache/kafka-native:4.4.0` ships, and can be dropped after that bump.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.

ayushtkn commented on code in PR #5839:
URL: https://github.com/apache/polaris/pull/5839#discussion_r4345474399


##########
extensions/events/kafka/src/test/java/org/apache/polaris/extensions/events/kafka/KafkaEventListenerTest.java:
##########
@@ -65,12 +65,17 @@
 @Testcontainers
 @ExtendWith(MockitoExtension.class)
 class KafkaEventListenerTest {
+  // apache/kafka-native images built with GraalVM < 25 segfault in getpwuid() on about 2% of
+  // starts (KAFKA-20314). Testcontainers only notices once the 60 second log wait times out, so
+  // allow more attempts; each starts a new container. Remove once the image is 4.4.0 or later.
+  // See https://github.com/apache/polaris/issues/5845.

Review Comment:
   I am not sure if we need to reference the issue here, git annotate can help reach the issue if it is link in the PR.
   
   I was checking the code no java file has a reference to the issue being fixed, maybe we can remove this // see line



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Retry the Kafka container start in KafkaEventListenerTest [polaris]

Posted by "erikwright (via GitHub)" <gi...@apache.org>.

erikwright commented on PR #5839:
URL: https://github.com/apache/polaris/pull/5839#issuecomment-6373259283

   > One small fix in the PR description: it says the floci containers use three attempts, but they use 5 (e.g. `FlociAwsExtension.java:102`). The one using 3 is `HiveCatalogFederationIntegrationTest`. Can we fix that line?
   
   Done.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org