Skip to content

KAFKA-1387: Kafka getting stuck creating ephemeral node it has already created when two zookeeper sessions are established in a very short period of time#178

Closed
fpj wants to merge 35 commits intoapache:trunkfrom
fpj:1387

Conversation

@fpj
Copy link
Copy Markdown
Contributor

@fpj fpj commented Aug 29, 2015

This is a patch to get around the problem discussed in the KAFKA-1387 jira. The tests are not passing in my box when I run them all, but they do pass when I run them individually, which indicates that there is something leaking from a test to the next. I still need to work this out and also work on further testing this. I wanted to open this PR now so that it can start getting reviewed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to remove this commented code.

@asfbot
Copy link
Copy Markdown

asfbot commented Aug 29, 2015

kafka-trunk-git-pr #267 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 2, 2015

kafka-trunk-git-pr #307 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 2, 2015

kafka-trunk-git-pr #308 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 2, 2015

kafka-trunk-git-pr #309 FAILURE
Looks like there's a problem with this pull request

@guozhangwang
Copy link
Copy Markdown
Contributor

@fpj Is this ready for review? Could you rebase?

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 3, 2015

kafka-trunk-git-pr #343 FAILURE
Looks like there's a problem with this pull request

@fpj
Copy link
Copy Markdown
Contributor Author

fpj commented Sep 3, 2015

@guozhangwang you can start reviewing if you have time. I'm primarily interested in your opinion about the approach of using a zk handle directly to solve this. I think it is ok, but creates some redudancy in ZkUtils.

Things that I'm not entirely happy with about the patch:
1- Upon a connectionloss event, it'd be better to wait until a connected event before retrying. The way it is isn't wrong, but it will be looping until it reconnects.
2- The current tests cover a lot of the functionality, so I think it is not worse compared to the existing trunk code, but I want to spend a bit more time thinking about test coverage.
3- I'm thinking that we should remove ZkUtils.createEphemeralPathExpectConflictHandleZKBug.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem that we need timeout any more. We will also need to remove it from the javadoc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 24, 2015

kafka-trunk-git-pr #512 FAILURE
Looks like there's a problem with this pull request

@fpj
Copy link
Copy Markdown
Contributor Author

fpj commented Sep 24, 2015

Test failures don't seem to be related to this patch:

kafka.consumer.MetricsTest > testMetricsLeak FAILED
kafka.api.ProducerBounceTest > testBrokerFailure FAILED
kafka.producer.ProducerTest > testSendWithDeadBroker FAILED

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after first comma

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 24, 2015

kafka-trunk-git-pr #526 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link
Copy Markdown

asfbot commented Sep 24, 2015

kafka-trunk-git-pr #528 FAILURE
Looks like there's a problem with this pull request

@junrao
Copy link
Copy Markdown
Contributor

junrao commented Sep 24, 2015

Thanks for the patch. LGTM.

@asfgit asfgit closed this in 1daf6ac Sep 24, 2015
jsancio pushed a commit to jsancio/kafka that referenced this pull request Aug 6, 2019
* CPKAFKA-2468: Make ccloud metadata watcher more reliable
@chia7712 chia7712 mentioned this pull request Dec 23, 2024
3 tasks
davide-armand pushed a commit to aiven/kafka that referenced this pull request Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants