You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "askwang (via GitHub)" <gi...@apache.org> on 2025/03/17 03:30:26 UTC

[PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

askwang opened a new pull request, #5297:
URL: https://github.com/apache/paimon/pull/5297

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   When reduce `scan.manifest.parallelism` parameter, it does not take effect.
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #5297:
URL: https://github.com/apache/paimon/pull/5297#discussion_r2002661608


##########
paimon-core/src/main/java/org/apache/paimon/utils/ManifestReadThreadPool.java:
##########
@@ -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()) {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #5297: [hotfix] Fix reduce scan.manifest.parallelism not take effect
URL: https://github.com/apache/paimon/pull/5297


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #5297:
URL: https://github.com/apache/paimon/pull/5297#discussion_r2002487144


##########
paimon-core/src/main/java/org/apache/paimon/utils/ManifestReadThreadPool.java:
##########
@@ -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()) {

Review Comment:
   Why do you need to reduce the number of threads?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #5297:
URL: https://github.com/apache/paimon/pull/5297#issuecomment-2964996132

   @askwang Hi, can you take a look to this PR: https://github.com/apache/paimon/pull/5737


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "askwang (via GitHub)" <gi...@apache.org>.
askwang commented on code in PR #5297:
URL: https://github.com/apache/paimon/pull/5297#discussion_r1998003783


##########
paimon-core/src/main/java/org/apache/paimon/utils/ManifestReadThreadPool.java:
##########
@@ -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()) {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #5297:
URL: https://github.com/apache/paimon/pull/5297#discussion_r1997907470


##########
paimon-core/src/main/java/org/apache/paimon/utils/ManifestReadThreadPool.java:
##########
@@ -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()) {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [hotfix] Fix reduce scan.manifest.parallelism not take effect [paimon]

Posted by "askwang (via GitHub)" <gi...@apache.org>.
askwang commented on code in PR #5297:
URL: https://github.com/apache/paimon/pull/5297#discussion_r2002632584


##########
paimon-core/src/main/java/org/apache/paimon/utils/ManifestReadThreadPool.java:
##########
@@ -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()) {

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org