Skip to content

MINOR: Change Trogdor agent's cleanup executor to a cached thread pool#6309

Merged
hachikuji merged 3 commits intoapache:trunkfrom
stanislavkozlovski:patch-4
Mar 12, 2019
Merged

MINOR: Change Trogdor agent's cleanup executor to a cached thread pool#6309
hachikuji merged 3 commits intoapache:trunkfrom
stanislavkozlovski:patch-4

Conversation

@stanislavkozlovski
Copy link
Copy Markdown
Contributor

It is best to use a growing thread pool for worker cleanups. This lets us ensure that we close workers as fast as possible and not get slowed down on blocking cleanups.

stanislavkozlovski and others added 2 commits February 22, 2019 11:57
It is best to use a growing thread pool for worker cleanups. This lets us ensure that we close workers as fast as possible and not get slowed down on blocking cleanups.
@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Feb 23, 2019

retest this please

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Feb 23, 2019

LGTM

@stanislavkozlovski
Copy link
Copy Markdown
Contributor Author

SpotBugs issued the following warning:


Exceptional return value of java.util.concurrent.ExecutorService.submit(Callable) ignored in org.apache.kafka.trogdor.agent.WorkerManager$Worker.transitionToStopping()
--
  | Bug type RV_RETURN_VALUE_IGNORED_BAD_PRACTICE (click for details) In class org.apache.kafka.trogdor.agent.WorkerManager$WorkerIn method org.apache.kafka.trogdor.agent.WorkerManager$Worker.transitionToStopping()Called method java.util.concurrent.ExecutorService.submit(Callable)At WorkerManager.java:[line 299

It apparently doesn't like executor.submit()'s return value be ignored. I made the transitionToStopping() method return it and spotBugs seemed to pass

Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the patch.

@hachikuji hachikuji merged commit 9a384da into apache:trunk Mar 12, 2019
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* warn-apache-kafka/trunk: (41 commits)
  MINOR: Avoid double null check in KStream#transform() (apache#6429)
  KAFKA-7944: Improve Suppress test coverage (apache#6382)
  KAFKA-3522: add missing guards for TimestampedXxxStore (apache#6356)
  MINOR: Change Trogdor agent's cleanup executor to a cached thread pool (apache#6309)
  KAFKA-7976; Update config before notifying controller of unclean leader update (apache#6426)
  KAFKA-7801: TopicCommand should not be able to alter transaction topic partition count
  KAFKA-8091; Wait for processor shutdown before testing removed listeners (apache#6425)
  MINOR: Update delete topics zk path in assertion error messages
  KAFKA-7939: Fix timing issue in KafkaAdminClientTest.testCreateTopicsRetryBackoff
  KAFKA-7922: Return authorized operations in Metadata request response (KIP-430 Part-2)
  MINOR: Print usage when parse fails during console producer
  MINOR: fix Scala compiler warning (apache#6417)
  KAFKA-7288; Fix check in SelectorTest to wait for no buffered bytes (apache#6415)
  KAFKA-8065: restore original input record timestamp in forward() (apache#6393)
  MINOR: cleanup deprectaion annotations (apache#6290)
  KAFKA-3522: Add TimestampedWindowStore builder/runtime classes (apache#6173)
  KAFKA-8069; Fix early expiration of offsets due to invalid loading of expire timestamp (apache#6401)
  KAFKA-8070: Increase consumer startup timeout in system tests (apache#6405)
  KAFKA-8040: Streams handle initTransactions timeout (apache#6372)
  KAFKA-7980 - Fix timing issue in SocketServerTest.testConnectionRateLimit (apache#6391)
  ...
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
apache#6309)

It is best to use a growing thread pool for worker cleanups. This lets us ensure that we close workers as fast as possible and not get slowed down on blocking cleanups.

Reviewers: Colin McCabe <cmccabe@apache.org>, Jason Gustafson <jason@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants