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 2022/01/18 17:45:58 UTC
[GitHub] [iceberg] Zhangg7723 opened a new pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Zhangg7723 opened a new pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919
This PR fixes a log error for rewrite parameter max-file-size-bytes, when I set the target-file-size-bytes less than max-file-size-bytes, spark throwed a checkAgument exception "Cannot set max-file-size-bytes is greater than or equal to target-file-size-bytes, all files written will be larger than the threshold", it's puzzling, so I fix this error.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919#discussion_r788175984
##########
File path: core/src/main/java/org/apache/iceberg/actions/BinPackStrategy.java
##########
@@ -259,7 +259,7 @@ private void validateOptions() {
MIN_FILE_SIZE_BYTES, RewriteDataFiles.TARGET_FILE_SIZE_BYTES, minFileSize, targetFileSize);
Preconditions.checkArgument(targetFileSize < maxFileSize,
- "Cannot set %s is greater than or equal to %s, all files written will be larger than the threshold, %d >= %d",
Review comment:
I think this is incorrect for the equals case. Can you revert the message changes and swap the argument order?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919#discussion_r787247618
##########
File path: core/src/main/java/org/apache/iceberg/actions/BinPackStrategy.java
##########
@@ -259,7 +259,7 @@ private void validateOptions() {
MIN_FILE_SIZE_BYTES, RewriteDataFiles.TARGET_FILE_SIZE_BYTES, minFileSize, targetFileSize);
Preconditions.checkArgument(targetFileSize < maxFileSize,
- "Cannot set %s is greater than or equal to %s, all files written will be larger than the threshold, %d >= %d",
Review comment:
I think the correct fix is to reverse `MAX_FILE_SIZE_BYTES` and `RewriteDataFiles.TARGET_FILE_SIZE_BYTES`. Leaving "or equal to" doesn't work.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Zhangg7723 commented on a change in pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
Zhangg7723 commented on a change in pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919#discussion_r788295442
##########
File path: core/src/main/java/org/apache/iceberg/actions/BinPackStrategy.java
##########
@@ -259,7 +259,7 @@ private void validateOptions() {
MIN_FILE_SIZE_BYTES, RewriteDataFiles.TARGET_FILE_SIZE_BYTES, minFileSize, targetFileSize);
Preconditions.checkArgument(targetFileSize < maxFileSize,
- "Cannot set %s is greater than or equal to %s, all files written will be larger than the threshold, %d >= %d",
Review comment:
OK, I will follow your advice, swap the argument order.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919#issuecomment-1019545412
Thanks, @Zhangg7723! Looks good. I'll merge when tests are passing.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Zhangg7723 commented on a change in pull request #3919: Core: Fix the incorrectly log description in BinPackStrategy
Posted by GitBox <gi...@apache.org>.
Zhangg7723 commented on a change in pull request #3919:
URL: https://github.com/apache/iceberg/pull/3919#discussion_r787644367
##########
File path: core/src/main/java/org/apache/iceberg/actions/BinPackStrategy.java
##########
@@ -259,7 +259,7 @@ private void validateOptions() {
MIN_FILE_SIZE_BYTES, RewriteDataFiles.TARGET_FILE_SIZE_BYTES, minFileSize, targetFileSize);
Preconditions.checkArgument(targetFileSize < maxFileSize,
- "Cannot set %s is greater than or equal to %s, all files written will be larger than the threshold, %d >= %d",
Review comment:
If MAX_FILE_SIZE_BYTES equals to TARGET_FILE_SIZE_BYTES, argument check still works and throws this exception. Anyway, there is a mistake I forget to fix "%d >= %d" with parameters, I will reverse targetFileSize and maxFileSize later.
thanks for your comment
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org