Skip to content

Conversation

@ajithme
Copy link
Contributor

@ajithme ajithme commented Jan 23, 2020

What changes were proposed in this pull request?

In org.apache.spark.sql.execution.SubqueryBroadcastExec#relationFuture make a copy of org.apache.spark.SparkContext#localProperties and pass it to the execution thread in org.apache.spark.sql.execution.SubqueryBroadcastExec#executionContext

Why are the changes needed?

In Dynamic pruning feature, when executing SubqueryBroadcastExec, the relationFuture is evaluated via a separate thread. The threads inherit the localProperties from sparkContext as they are the child threads.
These threads are created in the executionContext (thread pools). Each Thread pool has a default keepAliveSeconds of 60 seconds for idle threads.
Scenarios where the thread pool has threads which are idle and reused for a subsequent new query, the thread local properties will not be inherited from spark context (thread properties are inherited only on thread creation) hence end up having old or no properties set. This will cause SubqueryBroadcastExec#executionContext local properties to be missing or stale

Does this PR introduce any user-facing change?

No

How was this patch tested?

WIP

@ajithme
Copy link
Contributor Author

ajithme commented Jan 23, 2020

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@dongjoon-hyun
Copy link
Member

Do you think we can add a test case?

@ajithme ajithme changed the title [WIP][SPARK-30621][SQL] Dynamic Pruning thread propagates the localProperties to task [WIP][SPARK-30621][SQL] Dynamic Pruning thread propagates the localProperties to executionContext Jan 26, 2020
@ajithme
Copy link
Contributor Author

ajithme commented Jan 26, 2020

Do you think we can add a test case?

I investigate this and see that
Step1 : actual child of SubqueryBroadcastExec is a BroadcastExchangeExec, which gets evaluated in its own execution context i.e BroadcastExchangeExec#executionContext (we need to have #27266 to fix BroadcastExchangeExec first) already PR is available with testcase

Step 2 : Next, in SubqueryBroadcastExec#executionContext we just copy this hashkeys (obtained after evaluating relation in Step 1) can be used for dynamic pruning. Currently it do not use any thread local properties to do operations here, but as a safety its better to copy localProperties to SubqueryBroadcastExec#executionContext , this we cannot have a testcase as its not used now and more of a code review point.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label May 16, 2020
@github-actions github-actions bot closed this May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants