Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement]: Kafka Module: graceful shutdown #2206

Open
JordanP opened this issue Feb 12, 2024 · 5 comments
Open

[Enhancement]: Kafka Module: graceful shutdown #2206

JordanP opened this issue Feb 12, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@JordanP
Copy link
Contributor

JordanP commented Feb 12, 2024

Proposal

Currently, the Kafka module spawns a container that doesn't properly handle signals (term/int). The following (next) unit test reproduces the issue:

func TestKafkaGracefulShutdown(t *testing.T) {
	ctx := context.Background()
	kafkaContainer, err := RunContainer(ctx, WithClusterID("kraftCluster"), testcontainers.WithImage("confluentinc/confluent-local:7.5.0"))
	if err != nil {
		t.Fatal(err)
	}

	done := make(chan struct{})
	go func() {
		// Wait 60 seconds, after that the pod will be forcefully terminated.
		timeout := 60 * time.Second
		kafkaContainer.Stop(context.Background(), &timeout)
		done <- struct{}{}
	}()
	select {
	case <-done:
	case <-time.After(50 * time.Second): // 50 secs should be plenty for Kafka to properly handle the signal
		t.Fatalf("Kafka did not gracefully exit in due time")
	}
}

I tried to replace /etc/confluent/docker/launch with exec /etc/confluent/docker/launch and starterScript with exec starterScript but that didn't do the trick.

@JordanP JordanP added the enhancement New feature or request label Feb 12, 2024
@JordanP
Copy link
Contributor Author

JordanP commented Feb 12, 2024

Interestingly, using confluentinc/cp-kafka:7.5.3 and exec /etc/confluent/docker/launch and exec " + starterScript work just fine.

@mdelapenya
Copy link
Collaborator

Summoning @eddumelendez as our Kafka expert 🙏

@eddumelendez
Copy link
Member

It could be a difference between both images cp-kafka and confluent-local images. You can raise an issue here. I also found this old issue but it is just for cp-kafka

@JordanP
Copy link
Contributor Author

JordanP commented Feb 25, 2024

Do you remember/know why this Kafka module uses the confluent-local variant and not the (more widely used / less experimental ?) cp-kafka ?

@eddumelendez
Copy link
Member

I think the same implementation will work with confluentinc/cp-kafka but only in zookeeperless (raft) mode. IIRC, in go we decided to start with confluent-local as it is declared to be used for development workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants