Skip to content

refactor: Poll Context object for API #223

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Mar 23, 2022
Merged

refactor: Poll Context object for API #223

merged 20 commits into from
Mar 23, 2022

Conversation

astubbs
Copy link
Contributor

@astubbs astubbs commented Mar 10, 2022 •
edited
Loading

Provides:

  • an organised view per TopicPartition
  • failure counts for records - which will improve the retry system
  • Enables all api's to be batched - not just special batch methods
  • allow for future extensions without breaking primary APIs

This will be a minor breaking change

  • Replace work container look up through work manager with context object reference
  • Upgrade docs (breaking change)
  • Update docs (features and batching section)
  • Implement collection
  • Full failure history, not just counts - memory leak potential - cap at something reasonable (customisable)
  • Unify batch and non batch flows
  • Update retry provider
  • Hide work container access from public api through protected inheritance from internal package
  • Remove failure history to separate PR
  • Add record id(tp, offset) - useful for user to identify records.
  • For hiding work container, try out delegate instead of inheritance

@astubbs astubbs marked this pull request as draft March 10, 2022 21:12
@astubbs astubbs force-pushed the features/batching branch 4 times, most recently from b7b631c to 7e26075 Compare March 14, 2022 15:26
@astubbs astubbs force-pushed the features/context-object branch 2 times, most recently from 426c69b to 149a810 Compare March 15, 2022 00:01
@astubbs astubbs changed the base branch from features/batching to master March 15, 2022 00:02
@astubbs astubbs force-pushed the features/context-object branch from 149a810 to 50211f8 Compare March 15, 2022 13:45
This was referenced Mar 15, 2022
Closed
Merged
Closed
@astubbs astubbs force-pushed the features/context-object branch from a3f0705 to 6186f83 Compare March 15, 2022 20:47
@astubbs astubbs changed the base branch from master to features/shard-object March 15, 2022 20:56
astubbs
astubbs commented Mar 15, 2022
View reviewed changes
Copy link
Contributor Author

@astubbs astubbs left a comment

Choose a reason for hiding this comment

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

todos

This was referenced Mar 16, 2022
Merged
Open
@astubbs astubbs force-pushed the features/shard-object branch 2 times, most recently from 022a6ee to a940cfd Compare March 16, 2022 12:40
@astubbs astubbs force-pushed the features/context-object branch from f85e1bd to ea0a705 Compare March 16, 2022 12:55
Base automatically changed from features/shard-object to master March 16, 2022 13:08
astubbs added 3 commits March 16, 2022 13:11
@astubbs
START: Poll Context objects
d596cd2 
Provides an organised view per TopicPartition and includes failure counts for records.

step: BROKEN

step:

save

core save

step: COMPILES

step: review
@astubbs
step: review
fc3cd4f
@astubbs
step: review
e929438
@astubbs astubbs force-pushed the features/context-object branch from ea0a705 to e929438 Compare March 16, 2022 13:54
@astubbs
Copy link
Contributor Author

astubbs commented Mar 16, 2022

Thanks for the review @nioertel !

@astubbs astubbs force-pushed the features/context-object branch from 84c26d4 to 5c77f6d Compare March 23, 2022 12:06
astubbs
astubbs commented Mar 23, 2022
View reviewed changes
parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/PollContext.java
@Value(staticConstructor = "of")
@Getter(AccessLevel.NONE)
//@Setter(AccessLevel.NONE)
public class PollContext<K, V> implements Iterable<RecordContext<K, V>> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better name than PollContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not so far, but can change before the release...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nioertel ? @JorgenRingen ?

@astubbs
Copy link
Contributor Author

astubbs commented Mar 23, 2022

PR feature complete

astubbs
astubbs commented Mar 23, 2022
View reviewed changes
Copy link
Contributor Author

@astubbs astubbs left a comment

Choose a reason for hiding this comment

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

lgtm

astubbs
astubbs commented Mar 23, 2022
View reviewed changes
Copy link
Contributor Author

@astubbs astubbs left a comment

Choose a reason for hiding this comment

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

lgtm

@astubbs astubbs marked this pull request as ready for review March 23, 2022 12:27
@astubbs astubbs merged commit d31c7d3 into master Mar 23, 2022
@astubbs astubbs deleted the features/context-object branch March 23, 2022 12:40
This was referenced Mar 23, 2022
Closed
Closed
astubbs added a commit that referenced this pull request Mar 23, 2022
@astubbs
feature: Poll Context object for API (#223)
7c5ac01 
- Provides an organised view per TopicPartition and includes failure counts for records
- Unify batch and non-batch, record failure history
- Migrate to threeten's MutableClock
@astubbs astubbs mentioned this pull request Apr 19, 2022
Closed
acktsap
acktsap reviewed Nov 30, 2022
View reviewed changes
...onsumer-core/src/main/java/io/confluent/parallelconsumer/JStreamParallelStreamProcessor.java
public interface JStreamParallelStreamProcessor<K, V> extends DrainingCloseable {

static <KK, VV> JStreamParallelStreamProcessor<KK, VV> createJStreamEosStreamProcessor(ParallelConsumerOptions options) {
static JStreamParallelStreamProcessor createJStreamEosStreamProcessor(ParallelConsumerOptions<?, ?> options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why generic is removed in the factory method?

@acktsap acktsap mentioned this pull request Nov 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@nioertel nioertel nioertel left review comments

@acktsap acktsap acktsap left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@astubbs @nioertel @acktsap