refactor: merge threadpool and json-common modules by slievrly · Pull Request #8132 · apache/incubator-seata

slievrly · 2026-06-07T14:56:24Z

Eliminate two top-level modules (threadpool, json-common) by distributing their code into existing modules based on dependency constraints, and create dedicated jdk17/jdk21 modules for version-specific extensions.

threadpool refactoring:

Move SPI interface + platform impl (5 files) → common module
Move Factory + RuntimeEnvironment (2 files) → core module
Rename threadpool-loom → jdk21 (artifactId: seata-jdk21)
discovery-core/consul/etcd3/redis: use PlatformThreadPoolProvider directly to avoid core→discovery-core→core circular dependency

json-common refactoring:

Move JsonSerializer SPI + 4 impl classes + AllowlistManager → common
Move JsonUtil (depends on ConfigurationFactory) → core module
Rename json-common-jackson3 → jdk17 (artifactId: seata-jdk17)
Remove json-common-core dependency from 5 downstream modules (available transitively through core)

I have read the CONTRIBUTING.md guidelines.
I have registered the PR changes.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

…ture Eliminate two top-level modules (threadpool, json-common) by distributing their code into existing modules based on dependency constraints, and create dedicated jdk17/jdk21 modules for version-specific extensions. threadpool refactoring: - Move SPI interface + platform impl (5 files) → common module - Move Factory + RuntimeEnvironment (2 files) → core module - Rename threadpool-loom → jdk21 (artifactId: seata-jdk21) - discovery-core/consul/etcd3/redis: use PlatformThreadPoolProvider directly to avoid core→discovery-core→core circular dependency json-common refactoring: - Move JsonSerializer SPI + 4 impl classes + AllowlistManager → common - Move JsonUtil (depends on ConfigurationFactory) → core module - Rename json-common-jackson3 → jdk17 (artifactId: seata-jdk17) - Remove json-common-core dependency from 5 downstream modules (available transitively through core) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors the build/module layout by removing the top-level threadpool and json-common modules, relocating their code into common/core, and introducing jdk17/jdk21 extension modules to host JDK-specific SPI implementations.

Changes:

Moves thread pool SPI + default platform implementation into common, and factory/runtime selection into core; updates discovery modules to avoid circular dependencies by using PlatformThreadPoolProvider directly.
Moves JSON serializer SPI + implementations + allowlist manager into common, moves JsonUtil (configuration-aware) into core, and adds a JDK17-only Jackson3 serializer in jdk17.
Renames the loom module to jdk21 (seata-jdk21), updates Maven modules/profiles, and trims downstream POM dependencies accordingly.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
spring/seata-spring-autoconfigure/seata-spring-autoconfigure-client/pom.xml	Drops direct `json-common-core` dependency after refactor.
server/pom.xml	Switches JDK21 extension dependency from `seata-threadpool-loom` to `seata-jdk21`.
saga/pom.xml	Removes direct `json-common-core` dependency (now expected via core/common).
rm-datasource/pom.xml	Removes direct `json-common-core` dependency.
pom.xml	Removes `threadpool`/`json-common` modules and adds JDK17/JDK21 profile-driven modules.
jdk21/src/test/java/org/apache/seata/common/thread/VirtualThreadPoolProviderTest.java	Adds JDK21 virtual-thread provider tests.
jdk21/src/main/resources/META-INF/services/org.apache.seata.common.thread.ThreadPoolProvider	Registers the JDK21 `ThreadPoolProvider` SPI implementation.
jdk21/src/main/java/org/apache/seata/common/thread/VirtualThreadPoolProvider.java	Implements virtual-thread-backed `ThreadPoolProvider` for JDK21+.
jdk21/src/main/java/org/apache/seata/common/thread/VirtualThreadPoolExecutor.java	Adds virtual-thread-backed `ThreadPoolExecutor` implementation.
jdk21/src/main/java/org/apache/seata/common/thread/VirtualThreadFactoryHelper.java	Provides a virtual `ThreadFactory` helper enforcing daemon-only semantics.
jdk21/src/main/java/org/apache/seata/common/thread/VirtualScheduledThreadPoolExecutor.java	Adds virtual-thread-backed `ScheduledThreadPoolExecutor` implementation.
jdk21/pom.xml	Introduces the `seata-jdk21` module/artifact and dependencies.
jdk17/src/test/java/org/apache/seata/common/json/Jackson3JsonSerializerTest.java	Adds tests for the Jackson3 JSON serializer SPI.
jdk17/src/test/java/org/apache/seata/common/json/Jackson3AllowlistTest.java	Adds allowlist/security behavior tests for Jackson3 auto-type handling.
jdk17/src/main/resources/META-INF/services/org.apache.seata.common.json.JsonSerializer	Registers the JDK17 Jackson3 `JsonSerializer` SPI implementation.
jdk17/src/main/java/org/apache/seata/common/json/impl/Jackson3JsonSerializer.java	Adds Jackson 3.x `JsonSerializer` implementation with allowlist validation.
jdk17/pom.xml	Introduces the `seata-jdk17` module/artifact for JDK17+ extensions.
integration-tx-api/pom.xml	Drops direct `json-common-core` dependency.
discovery/seata-discovery-redis/src/main/java/org/apache/seata/discovery/registry/redis/RedisRegistryServiceImpl.java	Uses `PlatformThreadPoolProvider` directly to avoid dependency cycles.
discovery/seata-discovery-etcd3/src/main/java/org/apache/seata/discovery/registry/etcd3/EtcdRegistryServiceImpl.java	Uses `PlatformThreadPoolProvider` directly for executor construction.
discovery/seata-discovery-core/src/main/java/org/apache/seata/discovery/registry/RegistryHeartBeats.java	Uses `PlatformThreadPoolProvider` directly for heartbeat scheduler creation.
discovery/seata-discovery-core/pom.xml	Removes dependency on removed `seata-threadpool` module.
discovery/seata-discovery-consul/src/main/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImpl.java	Uses `PlatformThreadPoolProvider` directly for notifier executor construction.
core/src/test/java/org/apache/seata/common/thread/ThreadPoolExecutorFactoryTest.java	Adds tests for factory behavior and runtime selection logic.
core/src/main/java/org/apache/seata/common/thread/ThreadPoolRuntimeEnvironment.java	Adds runtime resolution for platform vs virtual thread pool modes.
core/src/main/java/org/apache/seata/common/thread/ThreadPoolExecutorFactory.java	Centralizes thread pool creation and provider selection in `core`.
core/src/main/java/org/apache/seata/common/json/JsonUtil.java	Adds configuration-aware JSON utility in `core`.
core/pom.xml	Removes dependency on removed `seata-threadpool` module.
compatible/pom.xml	Drops direct `json-common-core` dependency.
common/src/main/resources/META-INF/services/org.apache.seata.common.thread.ThreadPoolProvider	Registers the default platform thread pool provider SPI.
common/src/main/resources/META-INF/services/org.apache.seata.common.json.JsonSerializer	Registers built-in JSON serializer SPI implementations.
common/src/main/java/org/apache/seata/common/thread/ThreadPoolType.java	Adds enum for supported thread pool modes.
common/src/main/java/org/apache/seata/common/thread/ThreadPoolProviderOrders.java	Adds shared order constants for providers.
common/src/main/java/org/apache/seata/common/thread/ThreadPoolProvider.java	Introduces SPI interface for thread pool creation.
common/src/main/java/org/apache/seata/common/thread/PlatformThreadPoolProvider.java	Adds default platform-thread provider implementation.
common/src/main/java/org/apache/seata/common/thread/PlatformThreadPoolExecutor.java	Adds platform-thread-backed executor implementation wrapper.
common/src/main/java/org/apache/seata/common/json/JsonSerializerFactory.java	Adds central serializer factory with Jackson3 fallback behavior.
common/src/main/java/org/apache/seata/common/json/JsonSerializer.java	Adds unified JSON serializer SPI interface.
common/src/main/java/org/apache/seata/common/json/JsonAllowlistManager.java	Adds allowlist manager used by serializers to enforce safe polymorphic deserialization.
common/src/main/java/org/apache/seata/common/json/impl/JacksonJsonSerializer.java	Moves/hosts Jackson serializer implementation under `common`.
common/src/main/java/org/apache/seata/common/json/impl/GsonJsonSerializer.java	Moves/hosts Gson serializer implementation under `common`.
common/src/main/java/org/apache/seata/common/json/impl/FastjsonJsonSerializer.java	Moves/hosts Fastjson serializer implementation under `common`.
common/src/main/java/org/apache/seata/common/json/impl/Fastjson2JsonSerializer.java	Moves/hosts Fastjson2 serializer implementation under `common`.
common/pom.xml	Adds provided JSON library dependencies required by moved serializers.
bom/pom.xml	Replaces old threadpool artifacts with `seata-jdk21` in dependency management.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-06-07T15:24:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.38%. Comparing base (6ac9931) to head (c42648c).
⚠️ Report is 5 commits behind head on 2.x.

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x    #8132      +/-   ##
============================================
- Coverage     73.04%   72.38%   -0.66%     
+ Complexity     1142     1141       -1     
============================================
  Files          1151     1151              
  Lines         42272    42275       +3     
  Branches       5045     5045              
============================================
- Hits          30876    30600     -276     
- Misses         8921     9201     +280     
+ Partials       2475     2474       -1

Files with missing lines	Coverage Δ
...apache/seata/common/json/JsonAllowlistManager.java	`0.00% <ø> (ø)`
...pache/seata/common/json/JsonSerializerFactory.java	`50.00% <ø> (ø)`
...eata/common/json/impl/Fastjson2JsonSerializer.java	`12.06% <ø> (ø)`
...seata/common/json/impl/FastjsonJsonSerializer.java	`25.86% <ø> (ø)`
...che/seata/common/json/impl/GsonJsonSerializer.java	`0.00% <ø> (ø)`
.../seata/common/json/impl/JacksonJsonSerializer.java	`44.30% <ø> (ø)`
...eata/common/thread/PlatformThreadPoolExecutor.java	`100.00% <ø> (ø)`
...eata/common/thread/PlatformThreadPoolProvider.java	`100.00% <ø> (ø)`
...org/apache/seata/common/thread/ThreadPoolType.java	`71.42% <ø> (ø)`
...in/java/org/apache/seata/common/json/JsonUtil.java	`57.89% <ø> (ø)`
... and 15 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Fix hardcoded module name in warn log: "seata-threadpool-virtual" → "seata-jdk21" to match the renamed artifact - Add seata-jdk17 to bom/pom.xml dependency management so downstream builds using seata-bom can consume the Jackson3 extension Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@deprecated

…modules - Rename directories jdk17 → spi-jdk17, jdk21 → spi-jdk21 - Rename artifactIds seata-jdk17 → seata-spi-jdk17, seata-jdk21 → seata-spi-jdk21 - Delete old modules: threadpool/, threadpool-loom/, json-common/ - Fix @deprecated javadoc references: "json-common-core module" → "seata-common module" - Update warn log module name to seata-spi-jdk21 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 44 out of 71 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

spi-jdk17/pom.xml:32

PR description says the modules were renamed to artifacts seata-jdk17/seata-jdk21, but the actual published artifactId here is seata-spi-jdk17 (and seata-spi-jdk21 elsewhere). Please align the PR description with the implemented artifactIds, or rename the artifacts consistently if seata-jdk17/seata-jdk21 is the intended public API.

 /**
- * @deprecated use {@link org.apache.seata.common.json.JsonUtil} in json-common-core module instead.
+ * @deprecated use {@link org.apache.seata.common.json.JsonUtil} in seata-common module instead.
 */


+    private static final ScheduledExecutorService HEARTBEAT_SCHEDULED = new PlatformThreadPoolProvider()
+            .newScheduledThreadPoolExecutor("seata-discovery-heartbeat", 1, true, new ThreadPoolExecutor.AbortPolicy());



…olve # Conflicts: # pom.xml

funky-eyes

The overall module consolidation direction looks reasonable, but I think two issues should be addressed before merging:

The complete json-common-core test suite was deleted instead of being moved with the production code. This removes coverage for serializer SPI loading, Jackson 3 fallback, configuration resolution, and, more importantly, JSON deserialization allowlist security behavior. Please move serializer/factory/allowlist tests to seata-common, and move JsonUtilTest plus its configuration resources to seata-core.
The discovery implementations now instantiate PlatformThreadPoolProvider directly. This bypasses ThreadPoolExecutorFactory and therefore ignores the configured transport.threadpool mode for heartbeat, Consul notifier, Etcd watcher, and Redis subscription/update executors. Please introduce a shared provider resolver that does not create a core/discovery dependency cycle, or explicitly document and test that these executors are intentionally platform-thread-only.

Additional issues:

The deprecated integration-tx-api JsonUtil Javadoc says the replacement is in seata-common, while the class is now provided by seata-core.
The PR description refers to seata-jdk17 and seata-jdk21, but the actual artifacts are seata-spi-jdk17 and seata-spi-jdk21.
Please verify whether the removed Maven coordinates have ever been published. If so, compatibility bridge artifacts or an explicit migration strategy are needed.
Please add the corresponding change entries and run dependency:analyze for modules whose direct json-common-core dependency was removed.

Copilot AI review requested due to automatic review settings June 7, 2026 14:56

Copilot started reviewing on behalf of slievrly June 7, 2026 14:56 View session

Copilot AI reviewed Jun 7, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/seata/common/thread/ThreadPoolExecutorFactory.java Outdated

Comment thread bom/pom.xml

slievrly and others added 3 commits June 7, 2026 23:29

style: apply spotless formatting

e4d479a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

funky-eyes added this to the 2.8.0 milestone Jun 23, 2026

funky-eyes added the type: refactor refactor module label Jun 23, 2026

Merge branch '2.x' into 2.x_26_06_06

ddd5458

funky-eyes requested a review from Copilot June 23, 2026 06:41

Copilot started reviewing on behalf of funky-eyes June 23, 2026 06:42 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

slievrly and others added 2 commits July 27, 2026 01:19

Merge remote-tracking branch 'upstream/2.x' into pr-8132-conflict-res…

3e502c4

…olve # Conflicts: # pom.xml

Merge branch '2.x' into 2.x_26_06_06

c42648c

funky-eyes reviewed Jul 28, 2026

View reviewed changes

Merge branch '2.x' into 2.x_26_06_06

ebfa3b6

Navigation Menu

Uh oh!

refactor: merge threadpool and json-common modules - #8132

refactor: merge threadpool and json-common modules#8132
slievrly wants to merge 8 commits into
apache:2.xfrom
slievrly:2.x_26_06_06

slievrly commented Jun 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

funky-eyes left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		private static final ScheduledExecutorService HEARTBEAT_SCHEDULED = new PlatformThreadPoolProvider()
		.newScheduledThreadPoolExecutor("seata-discovery-heartbeat", 1, true, new ThreadPoolExecutor.AbortPolicy());

Uh oh!

Conversation

slievrly commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

funky-eyes left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

slievrly commented Jun 7, 2026 •

edited

Loading

codecov Bot commented Jun 7, 2026 •

edited

Loading