[
https://issues.apache.org/jira/browse/KAFKA-9134?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16970665#comment-16970665
]
Sophie Blee-Goldman commented on KAFKA-9134:
--------------------------------------------
Hm, maybe the wording of this ticket could have been more carefully chosen but
I think we're on the same page. There's actually not that much code that needs
to differ in the future assignor's case, but the problem is it can currently
only call the entire method. We should break up the various steps in
StreamsPartitionAssignor#assign and let the future assignor override only what
actually changes when the version gets bumped, and let them share the massive
amounts of busy work in the middle between parsing the (versioned) subscription
and writing the (versioned) assignment.
I took a look through your PR and definitely agree it's moving things in the
right direction, but I'd still want to do more with the
StreamsPartitionAssignor to allow for sensible code sharing (I guess a more
accurate description would be, sensible
not-sharing-parts-of-code-that-should-be-different ;))
> 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)
