Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State commit / cleanup #9

Closed
wants to merge 7 commits into from

Conversation

guozhangwang
Copy link

@ymatsuda

  1. Task state commit can happen either when user called context.commit() during the processing of a record in any processor, or when the commit interval has reached. In the first case, the task will commit it state including local state store, consumed offset and produced record; in the latter case, the thread will just commit the states of all tasks it owns.

  2. Did not have the threshold on total #. buffered records across tasks, since after chatting to @hachikuji I feel poll(0)'s overhead is very minimal, and asking users to specify the threshold could be hard since there is already a config for per-partition max buffered records.

  3. Some minor fixes on metrics recording, state cleanup, etc.

public RecordQueue(TopicPartition partition, SourceNode source) {
this.partition = partition;
this.source = source;

this.fifoQueue = new ArrayDeque<>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move these trivial variable initializations to constructor?
Is it the recommended Kafka coding style? Just wondering.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really kafka conventions but just personal coding style.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an issue for moving tirivial initialization to constructor?

@ymatsuda
Copy link

Left a minor comment.
LGTM

@guozhangwang
Copy link
Author

@ymatsuda closing this PR and creating a new one after merging in refactored kstream.

#12

@guozhangwang guozhangwang deleted the UnitTests branch October 7, 2016 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants