[
https://issues.apache.org/jira/browse/KAFKA-9134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16969829#comment-16969829
]
John Roesler commented on KAFKA-9134:
-------------------------------------
Thanks for calling this out [~ableegoldman].
Recently, while chasing down some problems with the version probing test, I
actually developed the opinion that the blame is in the opposite direction...
Namely, that the FutureStreamsPartitionAssignor shares too much code with the
StreamsPartitionAssignor. Reading over the details of your thinking, though,
I'm wondering if these are really just two ways of saying the same thing.
Actually, I recently made some changes that sound directionally similar to what
you propose. Can you take a look at https://github.com/apache/kafka/pull/7649
(and the other changes in that PR) and comment on whether you think it
addressed (some of) your concerns in this ticket?
> Refactor the StreamsPartitionAssignor for more code sharing with the
> FutureStreamsPartitionAssignor
> ---------------------------------------------------------------------------------------------------
>
> Key: KAFKA-9134
> URL: https://issues.apache.org/jira/browse/KAFKA-9134
> Project: Kafka
> Issue Type: Improvement
> Components: streams
> Reporter: Sophie Blee-Goldman
> Priority: Major
>
> Frequently when fixing bugs in the StreamsPartitionAssignor, version probing,
> or other assignor-related matters, we make a change in the
> StreamsPartitionAssignor and re-run the version_probing_upgrade system test
> only to see that the issue is not fixed because we forgot to mirror that
> change in the FutureStreamsPartitionAssignor. Worse yet, we are making a new
> change or fixing something that doesn't directly affect version probing, so
> we update only the StreamsPartitionAssignor and don't even run the version
> probing test, and discover later the version probing system test has started
> failing. Then we often waste time digging through old changes just to
> discover it was just because we forgot to copy any changes to the
> StreamsUpgradeTest classes (includes also it's future version of the
> SubscriptionInfo and AssignmentInfo classes)
>
> We should refactor the StreamsPartitionAssignor so that the future version
> can rely more heavily on the original class. This will probably involve
> either or all of:
> * making the class's latestSupportedVersion configurable in the constructor,
> to be used only by the system test. this will probably also require making it
> configurable in some other classes such as SubscriptionInfo and AssignmentInfo
> * breaking up the methods such as onAssignment and assign so that the future
> assignor can call smaller pieces at a time – part of the original problem is
> that the normal assignor will throw an exception partway through these
> methods if the latest version is a "future" one, so we end up having to call
> the method, catch the exception, and recode the remainder of the work in that
> method
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
