Skip to content

State commit / cleanup#9

Closed
Guozhang Wang (guozhangwang) wants to merge 7 commits into
confluentinc:streamingfrom
guozhangwang:UnitTests
Closed

State commit / cleanup#9
Guozhang Wang (guozhangwang) wants to merge 7 commits into
confluentinc:streamingfrom
guozhangwang:UnitTests

Conversation

@guozhangwang

Copy link
Copy Markdown

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 Jason Gustafson (@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.

Copy link
Copy Markdown

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
Copy Markdown
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oh...

Copy link
Copy Markdown
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
Copy Markdown

Left a minor comment.
LGTM

@guozhangwang

Copy link
Copy Markdown
Author

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

#12

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