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

SAMZA-2493: Keep checkpoint manager consumer open for repeated polling #1327

Merged
merged 5 commits into from Apr 7, 2020

Conversation

bkonold
Copy link
Contributor

@bkonold bkonold commented Mar 25, 2020

Feature: Allow control of whether the underlying consumer backing a checkpoint manager will be left open (until stopped), or closed after the initial read of checkpoints

Changes: Introduces an internal config, injected by SamzaContainer, that is injected dependent on whether the container is standby or not, to control whether the container's checkpoint manager should have it's underlying consumer stopped after initial read or not.

Tests: Added a unit test each for the config set to true / false

API Changes: None
Upgrade Instructions: None
Usage Instructions: None

@bkonold
Copy link
Contributor Author

bkonold commented Mar 31, 2020

@bharathkk @xinyuiscool can you take a look over this?

Copy link
Contributor

@mynameborat mynameborat left a comment

Choose a reason for hiding this comment

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

Minor comment around the configuration. Since its an internal config, there isn't going to be configuration document and it will be useful to have a sentence or two describing its purpose.
Looks good otherwise.

@mynameborat
Copy link
Contributor

Do you plan to add tests? I noticed TBD in the description.

@bkonold
Copy link
Contributor Author

bkonold commented Apr 6, 2020

Do you plan to add tests? I noticed TBD in the description.

@mynameborat yeah, added them

Copy link
Contributor

@mynameborat mynameborat left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests.
Will merge it in once travis checks pass.

@mynameborat mynameborat merged commit b193d45 into apache:master Apr 7, 2020
@bkonold bkonold deleted the checkpoint_consumer branch June 10, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants