-
Notifications
You must be signed in to change notification settings - Fork 42
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
Generate a separate file to list bootstrap properties #1517
Conversation
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me> Fixes NVIDIA#1509 - Add a new report to generate appId-bootstrap.conf - Add more context to the AutoTuner results - Add a yaml file to initialize the list of the bootstrap entries
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
This is submitted as a draft until we decide if we want to address some of the issues in followups or not:
|
Signed-off-by: Partho Sarthi <psarthi@nvidia.com>
Signed-off-by: Partho Sarthi <psarthi@nvidia.com>
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
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 @amahussein! Some minor nits and questions.
General question: for FILL_IN_VALUE
recommendations, I don't see explanation in comments for why Autotuner cannot set it. Do we expect users to know a good starting value or they can omit/research it? I am just wondering what is the thought behind this change. Thanks!
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/BootstrapReport.scala
Show resolved
Hide resolved
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 @amahussein! Some minor nits and questions. General question: for
FILL_IN_VALUE
recommendations, I don't see explanation in comments for why Autotuner cannot set it. Do we expect users to know a good starting value or they can omit/research it? I am just wondering what is the thought behind this change. Thanks!
Thanks @cindyyuanjiang !
Yes, the part of explaining why each entry is not set is yet to be implemented.
This PR provides the skeleton to allow such improvements. Depending on the bandwidth and priorities, each tuning strategy needs to be re-visited to add such explanation to the user.
I discussed with @parthosa offline some future work that need to be done.
Since there is a large backlog of AutoTuner requests, I will leave it to him to issue followups as he finds suits the priorities.
I do not think that filing those ideas as issues now will benefit us because it will a swamp that makes it harder to navigate AutoTuner issues.
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/BootstrapReport.scala
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/TuningEntryTrait.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
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 @mattahrens
I addressed the comments and populated the descriptions.
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 @amahussein! LGTM.
Signed-off-by: Ahmed Hussein (amahussein) a@ahussein.me
Fixes #1509
appId-bootstrap.conf
resources/boostrap/tuningTable.yaml
[link])defaultSpark
value to initialize the property: this is common for spark propertiesspark.locality.wait
was not processed by the AutoTunerspark.rapids.sql.enabled
was not added to the recommended entriesconf spark.sql.adaptive.autoBroadcastJoinThreshold=[FILL_IN_VALUE]
"This pull request includes several changes to improve the tuning configuration and profiling tools in the project. The most important changes include the addition of a new tuning table, modifications to the
AutoTuner
class and related classes to use a newTuningEntryTrait
, and the introduction of a bootstrap report generation.Sample Output:
Example of bootstrap file generated as
rapids_4_spark_qualification_output/tuning/APPID-bootstrap.conf
Updates to Tuning Configuration:
tuningTable.yaml
with various Spark properties for tuning and functionality.Refactoring for Tuning Entry (formerly:
RecommendationEntry
):Profiler.scala
to includeTuningEntryTrait
.runAutoTuner
and related methods inProfiler.scala
to useTuningEntryTrait
instead ofRecommendedPropertyResult
. [1] [2]RecommendationEntry
class and replaced its usage withTuningEntryTrait
inAutoTuner.scala
. [1] [2] [3] [4] [5] [6] [7] [8] [9]New Feature for Bootstrap Report:
BootstrapReport
class to generate reports containing required and tuned configurations.QualificationAutoTunerRunner
to generate a bootstrap report after tuning. [1] [2]