Skip to content

[hotfix] Fix reduce scan.manifest.parallelism not take effect #5297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

askwang
Copy link
Contributor

@askwang askwang commented Mar 17, 2025

Purpose

When reduce scan.manifest.parallelism parameter, it does not take effect.

Tests

API and Format

Documentation

@@ -36,7 +36,7 @@ public class ManifestReadThreadPool {
createCachedThreadPool(Runtime.getRuntime().availableProcessors(), THREAD_NAME);

public static synchronized ThreadPoolExecutor getExecutorService(@Nullable Integer threadNum) {
if (threadNum == null || threadNum <= executorService.getMaximumPoolSize()) {
if (threadNum == null || threadNum >= executorService.getMaximumPoolSize()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want increase thread Num, this becomes not work now...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can remove check threadNum <= executorService.getMaximumPoolSize() condition? increase or reduce both ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to reduce the number of threads?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We find there are 64 manifest thread when doing full manifest merge, it takes up a lot of memory, we want to relieve it by reducing the thream num.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe when the thread number is less than max pool size, you can introduce a SemaphoredDelegatingExecutor to wrap shared Executor.

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.

2 participants