Skip to content

[AMORO-4269] Refactor Iceberg thread pool initialization and scope#4270

Open
xxubai wants to merge 6 commits into
apache:masterfrom
xxubai:refactor-iceberg-thread-pools
Open

[AMORO-4269] Refactor Iceberg thread pool initialization and scope#4270
xxubai wants to merge 6 commits into
apache:masterfrom
xxubai:refactor-iceberg-thread-pools

Conversation

@xxubai

@xxubai xxubai commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Why are the changes needed?

Amoro uses deprecated Iceberg configuration and thread-pool APIs, while the planning and commit pool scope is not explicit enough for future process-specific scan/plan pools.

Close #4269.

Brief change log

  • Replace deprecated SystemProperties and ThreadPools.newWorkerPool() usage.
  • Initialize self-optimizing planning and commit pools from AMS configuration with explicit process-wide scope.
  • Rename pool accessors and document their usage, leaving independent initialization available for future Iceberg process pools.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

    ./mvnw -pl amoro-ams -am -DskipTests compile

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@xxubai
xxubai marked this pull request as ready for review July 10, 2026 09:17
@github-actions github-actions Bot added the module:ams-server Ams server module label Jul 10, 2026
@xxubai

xxubai commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Can you help review the PR @Aireed , @zhongqishang , @czy006 , thanks

@czy006

czy006 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

getThreadPool is called from the scan path through IcebergTableFileScanHelper.scan(). Since production standalone optimizers do not call IcebergThreadPools.init(), this fallback can be executed once per scan. The warning is currently logged unconditionally, which may cause significant log I/O and CPU overhead under load, despite the Javadoc stating that the warning is logged only once per pool.

Could we restore per-pool warning deduplication, for example with ConcurrentHashMap.newKeySet(), or otherwise rate-limit this log? Please also add a regression test to verify that repeated fallback calls emit the warning only once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement]: Refactor Iceberg thread pool initialization and scope

2 participants