[
https://issues.apache.org/jira/browse/KAFKA-12569?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
A. Sophie Blee-Goldman updated KAFKA-12569:
-------------------------------------------
Description:
The TaskManagerTest uses the StateMachineTask which extends AbstractTask. It
tracks a handful of flags related to commits, such as commitNeeded,
commitPrepared, etc. There's some overlap with the flags that are tracked in
StreamTask, which means we have to manually keep these up-to-date and make sure
the flags are set/cleared in the same way any time we make changes to ensure
the tests reflect reality. We also seem to use some of these flags to infer
task handling indirectly, such as using commitNeeded to indicate that a task
has been closed dirty/clean or has been committed successfully. We should clean
this up a bit and try to maintain the specific meaning of each flag, with new
flags added where needed.
For example, we added a commitSuccessful flag in KAFKA-12523. We should review
the existing tests and substitute it in where appropriate, eg when we're trying
to infer whether `postCommit` was called for a task. See [TODO|
https://github.com/apache/kafka/pull/10407/files#diff-48bc1476f0437fd711093c7c80ce73eda10be0511705799b4248545c203d521dR3049]
With the above in place, we can move the clearing of the postCommit flag from
the revive() override to the close methods, to more accurately match what is
done in StreamTask. See
[TODO|https://github.com/apache/kafka/pull/10407/files#diff-48bc1476f0437fd711093c7c80ce73eda10be0511705799b4248545c203d521dR3154]
See also discussion in this thread:
https://github.com/apache/kafka/pull/10407/#discussion_r602940025
was:
The TaskManagerTest uses the StateMachineTask which extends AbstractTask. It
tracks a handful of flags related to commits, such as commitNeeded,
commitPrepared, etc. There's some overlap with the flags that are tracked in
StreamTask, which means we have to manually keep these up-to-date and make sure
the flags are set/cleared in the same way any time we make changes to ensure
the tests reflect reality. We also seem to use some of these flags to infer
task handling indirectly, such as using commitNeeded to indicate that a task
has been closed dirty/clean or has been committed successfully. We should clean
this up a bit and try to maintain the specific meaning of each flag, with new
flags added where needed.
For example, we added a commitSuccessful flag in KAFKA-12523. We should review
the existing tests and substitute it in where appropriate, eg when we're trying
to infer whether `postCommit` was called for a task.
With the above in place, we can move the clearing of the postCommit flag from
the revive() override to the close methods, to more accurately match what is
done in StreamTask.
See discussion in this thread:
https://github.com/apache/kafka/pull/10407/#discussion_r602940025
> Clean up usage of commit flags in StateMachineTask
> --------------------------------------------------
>
> Key: KAFKA-12569
> URL: https://issues.apache.org/jira/browse/KAFKA-12569
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
> Reporter: A. Sophie Blee-Goldman
> Priority: Major
> Labels: tests
>
> The TaskManagerTest uses the StateMachineTask which extends AbstractTask. It
> tracks a handful of flags related to commits, such as commitNeeded,
> commitPrepared, etc. There's some overlap with the flags that are tracked in
> StreamTask, which means we have to manually keep these up-to-date and make
> sure the flags are set/cleared in the same way any time we make changes to
> ensure the tests reflect reality. We also seem to use some of these flags to
> infer task handling indirectly, such as using commitNeeded to indicate that a
> task has been closed dirty/clean or has been committed successfully. We
> should clean this up a bit and try to maintain the specific meaning of each
> flag, with new flags added where needed.
> For example, we added a commitSuccessful flag in KAFKA-12523. We should
> review the existing tests and substitute it in where appropriate, eg when
> we're trying to infer whether `postCommit` was called for a task. See [TODO|
> https://github.com/apache/kafka/pull/10407/files#diff-48bc1476f0437fd711093c7c80ce73eda10be0511705799b4248545c203d521dR3049]
> With the above in place, we can move the clearing of the postCommit flag from
> the revive() override to the close methods, to more accurately match what is
> done in StreamTask. See
> [TODO|https://github.com/apache/kafka/pull/10407/files#diff-48bc1476f0437fd711093c7c80ce73eda10be0511705799b4248545c203d521dR3154]
> See also discussion in this thread:
> https://github.com/apache/kafka/pull/10407/#discussion_r602940025
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
