Skip to content

KAFKA-14500; [4/N] Add Timer interface#13708

Merged
dajac merged 4 commits intoapache:trunkfrom
jeffkbkim:KAFKA-14500-4
May 23, 2023
Merged

KAFKA-14500; [4/N] Add Timer interface#13708
dajac merged 4 commits intoapache:trunkfrom
jeffkbkim:KAFKA-14500-4

Conversation

@jeffkbkim
Copy link
Copy Markdown
Contributor

@jeffkbkim jeffkbkim commented May 11, 2023

Adds the Timer interface that will be used by the new group coordinator.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dajac dajac added the KIP-848 The Next Generation of the Consumer Rebalance Protocol label May 12, 2023
@kirktrue
Copy link
Copy Markdown
Contributor

Thanks @jeffkbkim!

Any chance you could add @see annotations to the class-level Javadoc that points to where this is used? I see the GitHub PR description states that it's used by the GroupMetadataManager. It's helpful (to me, at least) to see where interfaces like this fit into the bigger picture.

Thanks!

@dajac
Copy link
Copy Markdown
Member

dajac commented May 12, 2023

@jeffkbkim Could you update the interface as we discussed offline?

@jeffkbkim
Copy link
Copy Markdown
Contributor Author

@kirktrue thanks for the comment. I will do so when GroupMetadataManager is merged to trunk.

@dajac updated the interface.

@jeffkbkim jeffkbkim marked this pull request as ready for review May 15, 2023 15:20
Copy link
Copy Markdown
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

I left a few minor comments. Should we update the title of the PR as well?

/**
* An interface to schedule and cancel operations.
*/
@InterfaceStability.Unstable
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: We don't need this because it is an internal interface anyway.

* @param deadlineMs The deadline to expire the operation in milliseconds.
* @param operation The operation to perform.
*/
void schedule(String key, long deadlineMs, Runnable operation);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a second though, I also wonder if we should use a delay instead of a deadline. This may be easier to use. We could also use TimeUnit.

@jeffkbkim jeffkbkim changed the title KAFKA-14500; [4/N] Add purgatory interface KAFKA-14500; [4/N] Add Timer interface May 22, 2023
Copy link
Copy Markdown
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

LGTM

@dajac dajac merged commit 15f8705 into apache:trunk May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

KIP-848 The Next Generation of the Consumer Rebalance Protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants