You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/01/07 18:46:31 UTC

[GitHub] [incubator-iceberg] aokolnychyi opened a new issue #726: Improve multi-threading in BaseRewriteManifests

aokolnychyi opened a new issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726
 
 
   I think we should revisit multi-threading logic in `BaseRewriteManifests` as we are using a thread pool but all methods in `WriterWrapper` are synchronized.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Here, the thread pool used in `performRewrite` seems to be a shared one (https://github.com/apache/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ThreadPools.java#L60).
   
   If the same BaseRewriteManifests object's `apply` method was called twice at the same time, it is possible that the same WriterWrapper object was accessed by two different threads in this shared thread pool.
   
   A more complex way is to create a thread worker pool, where each worker has its own WriterWrapper inside.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Here, the thread pool used in `performRewrite` seems to be a shared one (https://github.com/apache/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ThreadPools.java#L60).
   
   If the same BaseRewriteManifests object's `apply` method was called twice at the same time, it is possible that the same WriterWrapper object was accessed by two different threads in this shared thread pool.
   
   If this won't happen, I think we can drop `synchronized` in WriterWrapper.
   Otherwise, an alternative is to add synchronization around `performRewrite` and drop  `synchronized` in WriterWrapper.
   
   A more complex way is to create a thread worker pool, where each worker has its own WriterWrapper inside.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Looks like we can drop drop synchronization in WriterWrapper by grouping the remainingManifest by partitionSpecId and then passing each sublist as the input to the thread pool. 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he commented on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he commented on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Here, the thread pool used in `performRewrite` seems to be a shared one (https://github.com/apache/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ThreadPools.java#L60).
   
   If the same BaseRewriteManifests object's `apply` method was called twice at the same time, it is possible that the same WriterWrapper object was accessed by two different threads in this shared thread pool.
   
   If this won't happen, I think we can drop `synchronized` in WriterWrapper.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi closed issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
aokolnychyi closed issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Here, the thread pool used in `performRewrite` seems to be a shared one (https://github.com/apache/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ThreadPools.java#L60).
   
   If the same BaseRewriteManifests object's `apply` method was called twice at the same time, it is possible that the same WriterWrapper object was accessed by two different threads in this shared thread pool.
   
   If this won't happen, I think we can drop `synchronized` in WriterWrapper.
   Otherwise, an alternative is to add synchronization around `performRewrite` and drop  `synchronized` in WriterWrapper.
   A more complex way is to create a thread worker pool, where each worker has its own WriterWrapper inside.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on issue #726: Improve multi-threading in BaseRewriteManifests
URL: https://github.com/apache/incubator-iceberg/issues/726#issuecomment-583285403
 
 
   @aokolnychyi  Here, the thread pool used in `performRewrite` seems to be a shared one (https://github.com/apache/incubator-iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ThreadPools.java#L60).
   
   If the same BaseRewriteManifests object's `apply` method was called twice at the same time, it is possible that the same WriterWrapper object was accessed by two different threads in this shared thread pool.
   
   If this won't happen, I think we can drop `synchronized` in WriterWrapper.
   Otherwise, an alternative is to add synchronization around `performRewrite` and drop  `synchronized` in WriterWrapper.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org