Re: [PR] docs: Update configuration guide to show optional configs [datafusion-comet]
andygrove merged PR #1524: URL: https://github.com/apache/datafusion-comet/pull/1524 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] docs: Update configuration guide to show optional configs [datafusion-comet]
andygrove commented on code in PR #1524: URL: https://github.com/apache/datafusion-comet/pull/1524#discussion_r1994275251 ## docs/source/user-guide/configs.md: ## @@ -73,6 +73,7 @@ Comet provides the following configuration settings. | spark.comet.expression.allowIncompatible | Comet is not currently fully compatible with Spark for all expressions. Set this config to true to allow them anyway. For more information, refer to the Comet Compatibility Guide (https://datafusion.apache.org/comet/user-guide/compatibility.html). | false | | spark.comet.memory.overhead.factor | Fraction of executor memory to be allocated as additional non-heap memory per executor process for Comet. | 0.2 | | spark.comet.memory.overhead.min | Minimum amount of additional memory to be allocated per executor process for Comet, in MiB. | 402653184b | +| spark.comet.memoryOverhead | The amount of additional memory to be allocated per executor process for Comet, in MiB. This config is optional. If this is not specified, it will be set to `spark.comet.memory.overhead.factor` * `spark.executor.memory`. This is memory that accounts for things like Comet native execution, Comet shuffle, etc. | | Review Comment: Thanks. I have a follow-on PR that updates these configs to refer to the tuning guide and I have (hopefully) improved the documentation there about configuring memory. https://github.com/apache/datafusion-comet/pull/1525 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] docs: Update configuration guide to show optional configs [datafusion-comet]
comphead commented on code in PR #1524: URL: https://github.com/apache/datafusion-comet/pull/1524#discussion_r1994124731 ## docs/source/user-guide/configs.md: ## @@ -73,6 +73,7 @@ Comet provides the following configuration settings. | spark.comet.expression.allowIncompatible | Comet is not currently fully compatible with Spark for all expressions. Set this config to true to allow them anyway. For more information, refer to the Comet Compatibility Guide (https://datafusion.apache.org/comet/user-guide/compatibility.html). | false | | spark.comet.memory.overhead.factor | Fraction of executor memory to be allocated as additional non-heap memory per executor process for Comet. | 0.2 | | spark.comet.memory.overhead.min | Minimum amount of additional memory to be allocated per executor process for Comet, in MiB. | 402653184b | +| spark.comet.memoryOverhead | The amount of additional memory to be allocated per executor process for Comet, in MiB. This config is optional. If this is not specified, it will be set to `spark.comet.memory.overhead.factor` * `spark.executor.memory`. This is memory that accounts for things like Comet native execution, Comet shuffle, etc. | | Review Comment: Perhaps add the example? like `100M` -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] docs: Update configuration guide to show optional configs [datafusion-comet]
codecov-commenter commented on PR #1524: URL: https://github.com/apache/datafusion-comet/pull/1524#issuecomment-2721909717 ## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/1524?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report Attention: Patch coverage is `50.0%` with `3 lines` in your changes missing coverage. Please review. > Project coverage is 57.82%. Comparing base [(`f09f8af`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`2e44985`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/2e449850fc88256089c2d82ca3cabc9ffe8c75fb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 75 commits behind head on main. | [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-comet/pull/1524?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines | |---|---|---| | [...src/main/scala/org/apache/comet/GenerateDocs.scala](https://app.codecov.io/gh/apache/datafusion-comet/pull/1524?src=pr&el=tree&filepath=spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fcomet%2FGenerateDocs.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9jb21ldC9HZW5lcmF0ZURvY3Muc2NhbGE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-comet/pull/1524?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Additional details and impacted files ```diff @@ Coverage Diff @@ ## main#1524 +/- ## + Coverage 56.12% 57.82% +1.69% - Complexity 976 985 +9 Files 119 122 +3 Lines 1174312199 +456 Branches 2251 2304 +53 + Hits 6591 7054 +463 + Misses 4012 3962 -50 - Partials 1140 1183 +43 ``` [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/1524?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). 🚀 New features to boost your workflow: - ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
