Skip to content

Comments

[improve][broker] Refactor method BrokerService#unloadServiceUnit#21463

Closed
mattisonchao wants to merge 2 commits intoapache:masterfrom
mattisonchao:improve/broker_service_unloadServiceUnit
Closed

[improve][broker] Refactor method BrokerService#unloadServiceUnit#21463
mattisonchao wants to merge 2 commits intoapache:masterfrom
mattisonchao:improve/broker_service_unloadServiceUnit

Conversation

@mattisonchao
Copy link
Member

Motivation

Refactor method BrokerService#unloadServiceUnit to improve exception handling, logging and method call.

Modifications

  • Add parameters NPE check
  • Ignore the exception of topicCreationFuture

We should ignore the exception from topic creation future. It will give the caller a precise exception related to topic closing.

  • throw TimeoutException when the method running timeout.

As a public method, we should let users decide what they want to do when the method runs timeout. Plus, I checked all the usage of this method and all of them ignore the exception directly. It shouldn't be a breaking change.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

@mattisonchao mattisonchao self-assigned this Oct 29, 2023
@mattisonchao mattisonchao added this to the 3.2.0 milestone Oct 29, 2023
@mattisonchao mattisonchao added type/refactor Code or documentation refactors. e.g. refactor code structure or methods to improve code readability area/broker ready-to-test labels Oct 29, 2023
@mattisonchao mattisonchao reopened this Oct 29, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs ready-to-test type/refactor Code or documentation refactors. e.g. refactor code structure or methods to improve code readability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants