KAFKA-9113: P5, StandbyTask Life Cycle#7
Merged
Conversation
3 tasks
mjsax
reviewed
Jan 30, 2020
| try { | ||
| restoreConsumer.unsubscribe(); | ||
| } catch (KafkaException e) { | ||
| } catch (final KafkaException e) { |
There was a problem hiding this comment.
Why did this not fail the build without the final qualifier? Do we have some exceptions for this style check that we should remove?
Owner
Author
There was a problem hiding this comment.
This file was not in any suppression files.. I did not know why it does not fail the build before.
mjsax
reviewed
Jan 30, 2020
| for (final TopicPartition partition : partitions) { | ||
| final SourceNode source = topology.source(partition.topic()); | ||
| final TimestampExtractor sourceTimestampExtractor = source.getTimestampExtractor() != null ? source.getTimestampExtractor() : defaultTimestampExtractor; | ||
| final TimestampExtractor timestampExtractor = source.getTimestampExtractor(); |
There was a problem hiding this comment.
We get the extractor from the source; should the var name not be sourceTimestampExtractor ?
Owner
Author
There was a problem hiding this comment.
Sounds good, I can do that.
mjsax
reviewed
Jan 30, 2020
| final SourceNode source = topology.source(partition.topic()); | ||
| final TimestampExtractor sourceTimestampExtractor = source.getTimestampExtractor() != null ? source.getTimestampExtractor() : defaultTimestampExtractor; | ||
| final TimestampExtractor timestampExtractor = source.getTimestampExtractor(); | ||
| final TimestampExtractor sourceTimestampExtractor = timestampExtractor != null ? source.getTimestampExtractor() : defaultTimestampExtractor; |
There was a problem hiding this comment.
This may or maynot be the source extractor -- it could also be the default extractor. Should we rename to timestampExtractor?
guozhangwang
added a commit
to apache/kafka
that referenced
this pull request
Feb 5, 2020
This PR is collaborated by Guozhang Wang and John Roesler. It is a significant tech debt cleanup on task management and state management, and is broken down by several sub-tasks listed below: Extract embedded clients (producer and consumer) into RecordCollector from StreamTask. guozhangwang#2 guozhangwang#5 Consolidate the standby updating and active restoring logic into ChangelogReader and extract out of StreamThread. guozhangwang#3 guozhangwang#4 Introduce Task state life cycle (created, restoring, running, suspended, closing), and refactor the task operations based on the current state. guozhangwang#6 guozhangwang#7 Consolidate AssignedTasks into TaskManager and simplify the logic of changelog management and task management (since they are already moved in step 2) and 3)). guozhangwang#8 guozhangwang#9 Also simplified the StreamThread logic a bit as the embedded clients / changelog restoration logic has been moved into step 1) and 2). guozhangwang#10 Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Bruno Cadonna <bruno@confluent.io>, Boyang Chen <boyang@confluent.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standby Task implementation based on John's work.
Committer Checklist (excluded from commit message)