You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "CalvinConfluent (via GitHub)" <gi...@apache.org> on 2025/02/18 18:28:20 UTC
[PR] Minor: update truncation test [kafka]
CalvinConfluent opened a new pull request, #18952:
URL: https://github.com/apache/kafka/pull/18952
Adjust the behavior accordingly for the KIP-966
--
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] MINOR: update truncation test [kafka]
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1966179174
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
Is the idea that it's simpler to create the topic with this configuration than set it later? If so then the answer is "we could still use this, but we don't want to"?
--
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] MINOR: update truncation test [kafka]
Posted by "CalvinConfluent (via GitHub)" <gi...@apache.org>.
CalvinConfluent commented on PR #18952:
URL: https://github.com/apache/kafka/pull/18952#issuecomment-2672189787
testBalancePartitionLeaders can pass in local.
--
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] MINOR: update truncation test [kafka]
Posted by "CalvinConfluent (via GitHub)" <gi...@apache.org>.
CalvinConfluent commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1966276275
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
Yes, it is simpler. The `unclean.leader.election.enable` has to be set before an event that triggers the election. Because I enforce the event sequence in this PR, it is too late to set the config in its original place. So we need to set the config earlier. The option I choose is to set it during the topic creation.
--
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] MINOR: update truncation test [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18952:
URL: https://github.com/apache/kafka/pull/18952#issuecomment-2679582515
I don't cherry-pick it to 4.0, as ELR is disabled by default in 4.0 (see #18954)
--
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] MINOR: update truncation test [kafka]
Posted by "CalvinConfluent (via GitHub)" <gi...@apache.org>.
CalvinConfluent commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1968100708
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
@chia7712 Thanks for the comment. Removed the `set_unclean_leader_election`
--
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] MINOR: update truncation test [kafka]
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe commented on PR #18952:
URL: https://github.com/apache/kafka/pull/18952#issuecomment-2675559965
@CalvinConfluent Still one question left to answer.
--
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] MINOR: update truncation test [kafka]
Posted by "CalvinConfluent (via GitHub)" <gi...@apache.org>.
CalvinConfluent commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1962165856
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
`set_unclean_leader_election` sets the `unclean.leader.election.enable` config to true. It is the same if we set this config when creating the topic.
Creating the topic with the config so that the unclean election can trigger by this [event](https://github.com/apache/kafka/pull/18952/files#diff-6cfdf3acb5e73d7f00b0319316f4c22fda76f0e7b5fff4c6bfdd2e395dc08498R115). In the old way, election happens [here](https://github.com/apache/kafka/pull/18952/files#diff-6cfdf3acb5e73d7f00b0319316f4c22fda76f0e7b5fff4c6bfdd2e395dc08498R127) which is no longer possible if use `wait_until` to wait for the unclean leader election.
--
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] MINOR: update truncation test [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1962052021
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
For my understanding, why can't we use this one any more?
--
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] MINOR: update truncation test [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18952:
URL: https://github.com/apache/kafka/pull/18952#discussion_r1966667621
##########
tests/kafkatest/tests/client/truncation_test.py:
##########
@@ -112,7 +117,10 @@ def none_consumed(this, consumer):
pre_truncation_pos = consumer.current_position(tp)
- self.kafka.set_unclean_leader_election(self.TOPIC)
Review Comment:
+1 to set the `unclean.leader.election.enable` earlier - otherwise, in ELR mode, `self.kafka.start_node(node2)` does not trigger the unclean election as `node2` is not in ISR.
BTW, it seems `set_unclean_leader_election` is not used anymore - should we remove it in this PR?
--
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] MINOR: update truncation test [kafka]
Posted by "CalvinConfluent (via GitHub)" <gi...@apache.org>.
CalvinConfluent commented on PR #18952:
URL: https://github.com/apache/kafka/pull/18952#issuecomment-2675332313
@dajac @cmccabe Can you help take a look?
--
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] MINOR: update truncation test [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #18952:
URL: https://github.com/apache/kafka/pull/18952
--
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