You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "pjl1070048431 (via GitHub)" <gi...@apache.org> on 2025/08/16 08:57:41 UTC
[PR] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
pjl1070048431 opened a new pull request, #20360:
URL: https://github.com/apache/kafka/pull/20360
Currently, the following validation checks are performed twice for each partition placement:
throwInvalidReplicationFactorIfNonPositive()
throwInvalidReplicationFactorIfTooFewBrokers()
throwInvalidReplicationFactorIfZero()
These checks are already performed in the public place() method before the partition placement loop begins. Since the cluster state and replication factor don't change during the placement operation, these checks only need to be performed once.
This redundant validation could be removed from the inner loop to improve performance, especially when placing many partitions.
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "pjl1070048431 (via GitHub)" <gi...@apache.org>.
pjl1070048431 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3211701725
I think either is fine. Let's keep the checks within the loop.
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "pjl1070048431 (via GitHub)" <gi...@apache.org>.
pjl1070048431 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3196993995
ok i will check 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
Re: [PR] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3194471124
@pjl1070048431 Could you please merge trunk to trigger QA again?
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "pjl1070048431 (via GitHub)" <gi...@apache.org>.
pjl1070048431 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3222459672
Just close the 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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3215018997
> I think either is fine. Let's keep the checks within the loop.
@pjl1070048431 Do you want to close this PR or add comments explaining the duplicate checks as I mentioned?
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "pjl1070048431 (via GitHub)" <gi...@apache.org>.
pjl1070048431 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3194839626
ok
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3211234157
https://github.com/apache/kafka/pull/10823/files#r646142461
@pjl1070048431 Could you please take a look at above comment? it makes sense to me since `RackList` could be access in the same package. Especially, [KIP-1203](https://cwiki.apache.org/confluence/display/KAFKA/KIP-1203%3A+Allow+to+configure+custom+%60ReplicaPlacer%60+implementation) could encourage developers to use `RackList`
Perhaps we could add comments explaining why we need those "redundant" validation instead. WDYT?
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "pjl1070048431 (via GitHub)" <gi...@apache.org>.
pjl1070048431 closed pull request #20360: KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer
URL: https://github.com/apache/kafka/pull/20360
--
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] KAFKA-19612: Remove Redundant Validation in StripedReplicaPlacer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20360:
URL: https://github.com/apache/kafka/pull/20360#issuecomment-3196039450
@pjl1070048431 Could you please check the CI failure?
--
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