Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-770] [NSE-774] Fix runtime issues on spark 3.2 #773

Merged
merged 22 commits into from
Mar 23, 2022

Conversation

PHILO-HE
Copy link
Collaborator

This PR includes some fixes for runtime issues on spark 3.2. Thanks Yuan's help in fixing the issues when AQE is turned on.

@PHILO-HE PHILO-HE changed the title Fix runtime issues on spark 3.2 [NSE-770] [NSE-774] Fix runtime issues on spark 3.2 Mar 17, 2022
@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/native-sql-engine/issues

Then could you also rename commit message and pull request title in the following format?

[NSE-${ISSUES_ID}] ${detailed message}

See also:

@github-actions
Copy link

#770

(that canEqual this) && super.equals(that)
case _ => false
}

// For spark3.2.
override protected def withNewChildInternal(newChild: SparkPlan): ColumnarBroadcastExchangeAdaptor =
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like no need to override here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, should remove it for spark 3.1 compatibility. Just fixed in a new commit.

@PHILO-HE PHILO-HE changed the title [NSE-770] [NSE-774] Fix runtime issues on spark 3.2 [NSE-770] [NSE-774] Fix runtime issues on spark 3.2 (Partial commits cherry-picked by #776) Mar 21, 2022
@PHILO-HE
Copy link
Collaborator Author

The above commits have been cherry-picked into gazelle 1.3.1 branch.

@@ -375,6 +375,12 @@ case class ColumnarPostOverrides() extends Rule[SparkPlan] {
var isSupportAdaptive: Boolean = true

def replaceWithColumnarPlan(plan: SparkPlan): SparkPlan = plan match {
case RowToColumnarExec(child: BroadcastQueryStageExec) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you also make a note here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added. Thanks!

case RowToColumnarExec(child: BroadcastQueryStageExec) =>
logDebug(s"$child Due to a fallback of BHJ inserted into plan." +
s" See above override in BroadcastQueryStageExec")
val localBroadcastXchg = child.plan.asInstanceOf[BroadcastExchangeExec]
Copy link
Collaborator

Choose a reason for hiding this comment

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

may need to guard this logic, like check if child.isInstanceOf[xxx] first

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in new commit. Thanks!

@zhouyuan
Copy link
Collaborator

Note: Spark 321 is ok to run on AQE + DPP with this patch

@PHILO-HE
Copy link
Collaborator Author

Just rebased the code.

@PHILO-HE PHILO-HE changed the title [NSE-770] [NSE-774] Fix runtime issues on spark 3.2 (Partial commits cherry-picked by #776) [NSE-770] [NSE-774] Fix runtime issues on spark 3.2 Mar 22, 2022
@zhouyuan zhouyuan merged commit d7cac91 into oap-project:master Mar 23, 2022
@weiting-chen weiting-chen added the bug Something isn't working label Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants