Skip to content
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

[GLUTEN-8018][CORE] Support adjusting stage resource profile dynamically #8209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zjuwangg
Copy link
Contributor

What changes were proposed in this pull request?

It's a follow PR of #8195 and demonstrate how to adjust resource profile through Rule.

How was this patch tested?

Unit test will be updated soon.

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

@github-actions github-actions bot added CORE works for Gluten Core VELOX CLICKHOUSE labels Dec 11, 2024
Copy link

#8018

Copy link

Run Gluten Clickhouse CI on x86

@zhztheplayer zhztheplayer changed the title [GLUTEN-8018]Introduce GlutenAutoAdjustStageResourceProfileRule to auto adjust stage resource profile [GLUTEN-8018] Introduce GlutenAutoAdjustStageResourceProfileRule to auto adjust stage resource profile Dec 12, 2024
@zhztheplayer
Copy link
Member

Thank you for splitting the PR into 2. Let's discuss in #8195 before moving forward to this one.

@PHILO-HE PHILO-HE changed the title [GLUTEN-8018] Introduce GlutenAutoAdjustStageResourceProfileRule to auto adjust stage resource profile [GLUTEN-8018][CORE] Support adjust stage resource profile dynamically Dec 17, 2024
@PHILO-HE PHILO-HE changed the title [GLUTEN-8018][CORE] Support adjust stage resource profile dynamically [GLUTEN-8018][CORE] Support adjusting stage resource profile dynamically Dec 17, 2024
Copy link

Run Gluten Clickhouse CI on x86

Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests in this PR? Since the feature seems functional to users after the change. Thanks!

Also, could change [CORE] to [VL] in PR title if this is not ready for CH use yet.

@@ -109,6 +109,7 @@ object VeloxRuleApi {
injector.injectFinal(c => RemoveGlutenTableCacheColumnarToRow(c.session))
injector.injectFinal(c => GlutenFallbackReporter(c.glutenConf, c.session))
injector.injectFinal(_ => RemoveFallbackTagRule())
injector.injectFinal(c => GlutenAutoAdjustStageResourceProfile(c.glutenConf, c.session))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the rule between RemoveGlutenTableCacheColumnarToRow and GlutenFallbackReporter? Thanks.

@@ -180,5 +181,6 @@ object VeloxRuleApi {
injector.injectPostTransform(c => RemoveGlutenTableCacheColumnarToRow(c.session))
injector.injectPostTransform(c => GlutenFallbackReporter(c.glutenConf, c.session))
injector.injectPostTransform(_ => RemoveFallbackTagRule())
injector.injectPostTransform(c => GlutenAutoAdjustStageResourceProfile(c.glutenConf, c.session))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE works for Gluten Core VELOX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants