You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ebyhr (via GitHub)" <gi...@apache.org> on 2025/04/16 02:39:08 UTC
[PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
ebyhr opened a new pull request, #12806:
URL: https://github.com/apache/iceberg/pull/12806
Fixes #12676
I could reproduce the issue with `@RepeatedTest` annotation. This option fixes the flaky tests.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2047126216
##########
build.gradle:
##########
@@ -414,6 +414,7 @@ project(':iceberg-data') {
useJUnitPlatform()
// Only for TestSplitScan as of Gradle 5.0+
maxHeapSize '1500m'
+ jvmArgs += '-XX:-OmitStackTraceInFastThrow'
Review Comment:
is there a particular reason why this is only being applied to `iceberg-data` and not around L230? Also I think this is good to apply to `integrationTest` as well, but I would add a small comment describing why it's being added to both places
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#issuecomment-2820901729
@ebyhr I checked locally and the `TestSerializedMetadata` test still failed for me when repeatedly testing it with this change (I typically run repeated tests using the JUnit Runner in Intellij instead of the Gradle runner and configure **repeat until failure** in the execution configuration). I've opened https://github.com/apache/iceberg/pull/12867 to address this so that we can get a new 1.9.0 RC out without those tests failing locally with JDK 17
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#issuecomment-2818218181
@Kontinuation Removing the message assertion makes sense for `TestSerializedMetadata.testEmptyVariantMetadata`, but I still recommend adding this option for other tests.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2053486396
##########
build.gradle:
##########
@@ -207,6 +207,11 @@ subprojects {
testImplementation libs.assertj.core
}
+ tasks.withType(Test).configureEach {
+ // Some tests verify the messages included in the stack traces
Review Comment:
I changed it the following sentence:
> JVM may stop outputting stack trace of the exception which often occurs.
> Disable the optimization because some tests depend on the stack trace.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2045903617
##########
build.gradle:
##########
@@ -414,6 +414,7 @@ project(':iceberg-data') {
useJUnitPlatform()
// Only for TestSplitScan as of Gradle 5.0+
maxHeapSize '1500m'
+ jvmArgs += '-XX:-OmitStackTraceInFastThrow'
Review Comment:
I'm not familiar with Gradle syntax. Let me know if there is a better way.
Also, I wonder if we should add the same argument to the `integrationTest` task.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2047916678
##########
build.gradle:
##########
@@ -414,6 +414,7 @@ project(':iceberg-data') {
useJUnitPlatform()
// Only for TestSplitScan as of Gradle 5.0+
maxHeapSize '1500m'
+ jvmArgs += '-XX:-OmitStackTraceInFastThrow'
Review Comment:
It was my oversight. I think we can use `tasks.withType(Test)` to avoid duplication.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2045903617
##########
build.gradle:
##########
@@ -414,6 +414,7 @@ project(':iceberg-data') {
useJUnitPlatform()
// Only for TestSplitScan as of Gradle 5.0+
maxHeapSize '1500m'
+ jvmArgs += '-XX:-OmitStackTraceInFastThrow'
Review Comment:
I'm not familiar with Gradle best practices. Let me know if there is a better way.
Also, I wonder if we should add the same argument to the `integrationTest` task.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#issuecomment-2818210775
I would suggest simply removing the assertion on the error message `.hasMessageContaining("out of bounds");`. The exact error message of the ArrayIndexOutOfBoundsException raised by indexing Java array is not guaranteed by the Java specification, and the error messages raised by JVM may change over time (see https://openjdk.org/jeps/358 as an example).
`.isInstanceOf(ArrayIndexOutOfBoundsException.class)` in the original test is already a proper validation of expected behavior. Removing the assertion on error message would make the test more robust against JVM implementation differences.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr closed pull request #12806: Build: Specify -XX:-OmitStackTraceInFastThrow in tests
URL: https://github.com/apache/iceberg/pull/12806
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2053465821
##########
build.gradle:
##########
@@ -207,6 +207,11 @@ subprojects {
testImplementation libs.assertj.core
}
+ tasks.withType(Test).configureEach {
+ // Some tests verify the messages included in the stack traces
Review Comment:
I don't think it's clear what this comment is trying to say and how it relates to the JVM arg
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#discussion_r2053486396
##########
build.gradle:
##########
@@ -207,6 +207,11 @@ subprojects {
testImplementation libs.assertj.core
}
+ tasks.withType(Test).configureEach {
+ // Some tests verify the messages included in the stack traces
Review Comment:
I changed it to the following sentence:
> JVM may stop outputting stack trace of the exception which often occurs.
> Disable the optimization because some tests depend on the stack trace.
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra closed pull request #12806: Build: Specify -XX:-OmitStackTraceInFastThrow in tests
URL: https://github.com/apache/iceberg/pull/12806
--
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
Re: [PR] Build: Specify -XX:-OmitStackTraceInFastThrow in tests [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on PR #12806:
URL: https://github.com/apache/iceberg/pull/12806#issuecomment-2808102126
CI hit #11047
--
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