-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-40775: [Benchmarking][Java] Fix conbench timeout #40786
GH-40775: [Benchmarking][Java] Fix conbench timeout #40786
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format?
or
In the case of PARQUET issues on JIRA the title also supports:
See also: |
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 17abf85. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 17abf85. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
dev/conbench_envs/benchmarks.env
Outdated
@@ -48,3 +48,4 @@ PARQUET_BUILD_EXAMPLES=ON | |||
PARQUET_BUILD_EXECUTABLES=ON | |||
PYTHON=python | |||
LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH | |||
JAVA_OPTIONS=-Xmx8G |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danepitkin just curious, should we set Xms
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, would it be possible to make this CLI args? Just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Xms, I don't think it's necessary.
This can be passed in as cli args, too! We can add it as --java-options -Xmx8G
in a different repo here: https://github.com/voltrondata-labs/benchmarks/blob/231078e715001c32d2f4ef2ffca27abb4625fd21/benchmarks/java_micro_benchmarks.py#L47-L58
For now, I'm using this method just to test more easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for trying this out, @danepitkin 🙂 If it turns out that tweaking the environment configuration fixes the error, I think leaving that configuration here (in the dev/conbench_envs
location, which is only used by the Ursa machine) is the best choice.
Reason is we support running the voltrondata-labs/benchmarks
in other locations too, like local development, so I wouldn't want to always clobber the memory options of users' machines with a CLI arg there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me! Isolating the fix to the smallest scope is the preferred option IMO.
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 6658872. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 97df26b. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Just noting here that setting java max heap size to 8GB throws
but increasing max heap size to 16GB throws
|
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 6658872. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 3d98b4a. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 97df26b. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 3d98b4a. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
|
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 7fafa78. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Thanks for your patience. Conbench analyzed the 1 benchmarking run that has been run so far on PR commit 7fafa78. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit 7f2a480. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
@ursabot please benchmark lang=Java |
Benchmark runs are scheduled for commit a09d83c. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 7f2a480. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
Thanks for your patience. Conbench analyzed the 1 benchmarking run that has been run so far on PR commit a09d83c. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
result); | ||
result); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added to fix this bug that appeared in CI https://github.com/apache/arrow/actions/runs/8556425644/job/23446209816?pr=40786
@danepitkin mind updating the PR description now that the approach is different? |
Agh, good catch. Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 139afe5. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
### Rationale for this change The java build script has been recently updated and it is affecting conbench, which is now seeing timeouts when building java. The logs are producing 100s of GB of data due to an unnecessary debug log msg. ### What changes are included in this PR? * Delete log message on write to memory ### Are these changes tested? Yes, via conbench ### Are there any user-facing changes? No * GitHub Issue: apache#40775 Authored-by: Dane Pitkin <dane@voltrondata.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
### Rationale for this change The java build script has been recently updated and it is affecting conbench, which is now seeing timeouts when building java. The logs are producing 100s of GB of data due to an unnecessary debug log msg. ### What changes are included in this PR? * Delete log message on write to memory ### Are these changes tested? Yes, via conbench ### Are there any user-facing changes? No * GitHub Issue: apache#40775 Authored-by: Dane Pitkin <dane@voltrondata.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Rationale for this change
The java build script has been recently updated and it is affecting conbench, which is now seeing timeouts when building java. The logs are producing 100s of GB of data due to an unnecessary debug log msg.
What changes are included in this PR?
Are these changes tested?
Yes, via conbench
Are there any user-facing changes?
No
java.lang.OutOfMemoryError
in Java benchmarks after local build cache change #40775