Skip to content

[improve] [pip] PIP-387: Modify interface TopicCompactionService to support cancelling an in-progress compaction task #23462

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Oct 15, 2024

Motivation

Modifications

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 4.1.0 milestone Oct 15, 2024
@poorbarcode poorbarcode self-assigned this Oct 15, 2024
@github-actions github-actions bot added PIP doc-not-needed Your PR changes do not impact docs labels Oct 15, 2024

## Upgrade

If you did not implement a customized `TopicCompactionService` or `Compactor`, it is safety. Otherwise, you need to change your code.
Copy link
Contributor

@liangyepianzhou liangyepianzhou Oct 29, 2024

Choose a reason for hiding this comment

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

It seems to be a break change.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's acceptable in a new release. You can see TopicCompactionService is annotated with @InterfaceStability.Evolving.

  /**
   * Evolving, but can break compatibility at minor release (i.e. m.x)
   */
  @Documented
  public @interface Evolving {}

+ CompactionTaskCtx compact();
```

**CompactionTaskCtx.java**
Copy link
Contributor

Choose a reason for hiding this comment

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

This class exposes too many details. Why not just add a cancel() to the TopicCompactionService?

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

This PIP seems to aim at cancelling an in-progress compaction task. Just changing the interface is not enough. You should add an admin API for it if you want users to cancel a compaction task. Or you must mention when could a compaction task be cancelled if the cancellation is only performed internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants