Skip to content

Conversation

@mingri31164
Copy link
Contributor

@mingri31164 mingri31164 commented Sep 30, 2025

Problem

  1. Hard-coded ResizableCapacityLinkedBlockingQueue type check in ServerThreadPoolDynamicRefresh.changePoolInfo()
  2. Cannot support custom blocking queue implementations like rejection policy

Solution

  1. Replace hardcoded type check with capability-based detection
  2. Integrate CustomBlockingQueue SPI into BlockingQueueTypeEnum.createBlockingQueue()
  3. Add BlockingQueueManager as unified facade for queue operations
  4. Extract queue handling logic to separate handleQueueChanges() method
  5. Support custom queue type configuration (applied on client restart via reflection)
  6. Apply default capacity (1024) for custom queues

Key Changes

  1. Remove hardcoded type check in changePoolInfo():

    • Replace Objects.equals(RESIZABLE_LINKED_BLOCKING_QUEUE.getType(), parameter.getQueueType()) with capability detection
    • Extract to new handleQueueChanges() method for better separation
    • Use BlockingQueueManager.canChangeCapacity() for dynamic detection
  2. Add BlockingQueueManager unified facade:

    • Create queue by type ID (built-in + custom SPI)
    • Validate queue configuration for all types
    • Check capacity change capability dynamically
    • Adjust capacity at runtime
    • Recognize queue type for built-in and SPI queues
  3. Integrate CustomBlockingQueue SPI:

    • Enhance BlockingQueueTypeEnum.createBlockingQueue() to load SPI custom queues
    • Add customOrDefaultQueue() helper for SPI lookup with fallback
    • Register SPI via ServiceLoaderRegistry
  4. Add documentation:

    • queue-custom.md for SPI registration guide
    • queue-info.md for queue type lifecycle explanation

Test Coverage

  1. BlockingQueueSpiTest: 10 scenarios including SPI registration, capacity capability detection, and design consistency
  2. QueueSpiIntegrationTest: 6 scenarios validating hardcoded restriction removal and end-to-end flow
  3. ConfigServiceQueueCapacityTest: 11 scenarios for custom queue default capacity handling

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements a pluggable blocking queue SPI and a safe “rebuild-and-switch” path to dynamically change executor queue types without hardcoding, plus concurrency controls around thread-pool rebuilds.

  • Adds CustomBlockingQueue SPI and integrates SPI loading into BlockingQueueTypeEnum/BlockingQueueManager.
  • Introduces ThreadPoolRebuilder with per-pool locking, new-executor creation, task migration, atomic registry swap, and rollback.
  • Updates ServerThreadPoolDynamicRefresh to use the SPI/manager and adds capacity-change support where possible.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
starters/threadpool/server/src/main/java/cn/hippo4j/springboot/starter/core/ThreadPoolRebuilder.java New helper to rebuild executors with new queues, migrate tasks, switch registry, and handle rollback with locking.
starters/threadpool/server/src/main/java/cn/hippo4j/springboot/starter/core/ServerThreadPoolDynamicRefresh.java Uses BlockingQueueManager + ThreadPoolRebuilder to handle queue-type/capacity changes safely.
infra/common/src/main/java/cn/hippo4j/common/executor/support/BlockingQueueManager.java New manager for creation, validation, recognition, and capacity changes; includes a reflective queue-replace method.
infra/common/src/test/resources/META-INF/services/cn.hippo4j.common.executor.support.CustomBlockingQueue Registers a test SPI queue implementation for tests.
infra/common/src/test/java/cn/hippo4j/common/executor/support/ThreadPoolRebuilderConcurrencyTest.java Adds concurrency “design validation” tests (mostly prints) for rebuild scenarios.
infra/common/src/test/java/cn/hippo4j/common/executor/support/BlockingQueueSpiTest.java Adds SPI and manager unit tests for queue creation, validation, and recognition.
infra/common/src/test/java/cn/hippo4j/common/executor/integration/QueueSpiIntegrationTest.java Adds integration-style tests for SPI path (still mostly direct calls/prints).
docs/.../queue-info.md New docs page listing built-in queues and manager APIs.
docs/.../queue-custom.md New docs page on SPI for custom queues; includes code snippets.
Comments suppressed due to low confidence (1)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mingri31164 mingri31164 changed the title feat: Implement blocking queue SPI extension and safe dynamic switching feat: Implement blocking queue SPI extension Oct 28, 2025
@mingri31164 mingri31164 changed the title feat: Implement blocking queue SPI extension feat: Implement blocking queue SPI extension and add documentation Oct 28, 2025
@mingri31164
Copy link
Contributor Author

Hi @Createsequence ,

I have a question regarding the Mockito version upgrade in this PR.

Currently: Mockito 4.11.0 (for JDK 17 compatibility)
Previous: Mockito 3.12.4

Should we keep the upgrade to 4.11.0 for JDK 17 support, or should I revert it back to 3.12.4?

Thank you for your feedback!

@magestacks
Copy link
Member

Hi @Createsequence ,

I have a question regarding the Mockito version upgrade in this PR.

Currently: Mockito 4.11.0 (for JDK 17 compatibility) Previous: Mockito 3.12.4

Should we keep the upgrade to 4.11.0 for JDK 17 support, or should I revert it back to 3.12.4?

Thank you for your feedback!

@mingri31164 If the current version is not affected by this PR, keep the original version. The principle of a PR is to remain focused on a single purpose.

@Createsequence
Copy link
Collaborator

Createsequence commented Oct 29, 2025

Hi @mingri31164

I noticed you also modified the front-end code for the console, but after the server starts, the front-end page doesn't support switching custom blocking queues.

Please confirm that after packaging your front-end code, you replaced the original files under the path threadpool/server/console/src/main/resources/static.

@mingri31164
Copy link
Contributor Author

Hi @Createsequence ,

Thank you for your feedback. I'd like to clarify the situation:

Original Frontend Behavior

The original code already supports editing queue types in the admin console, including all built-in queue types (ArrayBlockingQueue, LinkedBlockingQueue, etc.). There was no restriction on changing queue types in the frontend.
44a3e41758dc060ecfaff10f7f4e7a33

My Changes

I followed the same pattern and added support for custom blocking queues:

  • Added option { key: 99, display_name: 'CustomBlockingQueue (SPI)' }
  • Added an input field for specifying the custom queue type ID
  • This mirrors the CustomRejectedPolicy (SPI) implementation
18b0300bbfc85666a33851d095d5a586

Queue Type Change Mechanism

Both built-in and custom queue type changes follow the same restart-based mechanism:

  • ✅ Configuration saved to database when edited
  • ❌ NOT pushed to running clients immediately
  • ✅ Takes effect only when client application restarts
  • ✅ Safe - no risk of task loss

Next Steps

If you think rebuilding and deploying the frontend is not necessary, I will revert the frontend changes to the previous version instead.

Please let me know if you'd like me to proceed. Thank you!

@Createsequence
Copy link
Collaborator

Hi @Createsequence ,

Thank you for your feedback. I'd like to clarify the situation:

Original Frontend Behavior

The original code already supports editing queue types in the admin console, including all built-in queue types (ArrayBlockingQueue, LinkedBlockingQueue, etc.). There was no restriction on changing queue types in the frontend. 44a3e41758dc060ecfaff10f7f4e7a33

My Changes

I followed the same pattern and added support for custom blocking queues:

  • Added option { key: 99, display_name: 'CustomBlockingQueue (SPI)' }
  • Added an input field for specifying the custom queue type ID
  • This mirrors the CustomRejectedPolicy (SPI) implementation
18b0300bbfc85666a33851d095d5a586 ### Queue Type Change Mechanism Both built-in and custom queue type changes follow the same restart-based mechanism:
  • ✅ Configuration saved to database when edited
  • ❌ NOT pushed to running clients immediately
  • ✅ Takes effect only when client application restarts
  • ✅ Safe - no risk of task loss

Next Steps

If you think rebuilding and deploying the frontend is not necessary, I will revert the frontend changes to the previous version instead.

Please let me know if you'd like me to proceed. Thank you!

Your solution is undoubtedly reasonable, and I don't think there's anything wrong with it.
Just considering that the front-end and back-end code are in the same project, I think the front-end and back-end code under the same theme should be submitted together, just like you did in this commit is done like that.
Anyway, this is a great PR with clean code and detailed comments. Once we finish the final review, we will merge it as soon as possible :)

@Createsequence Createsequence requested review from Createsequence and removed request for magestacks and pirme October 30, 2025 09:09
Copy link
Collaborator

@Createsequence Createsequence left a comment

Choose a reason for hiding this comment

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

good job

@Createsequence Createsequence merged commit 9aa3be0 into opengoofy:develop Oct 30, 2025
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants