pvary commented on a change in pull request #2228:
URL: https://github.com/apache/iceberg/pull/2228#discussion_r601457828



##########
File path: mr/src/main/java/org/apache/iceberg/mr/InputFormatConfig.java
##########
@@ -58,8 +58,11 @@ private InputFormatConfig() {
   public static final String LOCATION_PROVIDER = 
"iceberg.mr.location.provider";
   public static final String ENCRYPTION_MANAGER = 
"iceberg.mr.encription.manager";
 
-  public static final String COMMIT_THREAD_POOL_SIZE = 
"iceberg.mr.commit.thread.pool.size";
-  public static final int COMMIT_THREAD_POOL_SIZE_DEFAULT = 10;
+  public static final String OUTPUT_TABLES = "iceberg.mr.output.tables";
+  public static final String COMMIT_TABLE_THREAD_POOL_SIZE = 
"iceberg.mr.commit.table.thread.pool.size";

Review comment:
       > Overall, I don't think there are any blockers. I'd prefer to avoid the 
complexity of more thread pools and configuration constants, but it should be 
up to people that use Hive more than I do.
   
   I would prefer to keep the parallel commits because I think the complexity 
is around the same thanks to the `Tasks` API. Other than creating the executors 
it is just the matter of using `Tasks` instead of loops. On the other hand 
since we do not have atomicity in commits we are better of committing as fast 
as possible to prevent concurrent changes.
   
   I had to rebase and during the rebase I have realized that I have used the 
same `FileIO` to access the temp directory for every table. In the unit tests 
it was not an issue since all of the tables were using the same `io`, but I had 
to fix that.
   
   Thanks,
   Peter




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to