Skip to content

Conversation

@robert3005
Copy link

What changes were proposed in this pull request?

Make TaskContext reference an InheritableTheadLocal so thread pools spun up inside tasks have access to the reference

How was this patch tested?

Added tests

@ash211
Copy link
Contributor

ash211 commented Jun 2, 2017

Jenkins this is ok to test

@SparkQA
Copy link

SparkQA commented Jun 2, 2017

Test build #77681 has finished for PR 18176 at commit 06405ef.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@robert3005 robert3005 force-pushed the rk/inheritable-thread-local branch from 06405ef to 6430dca Compare June 5, 2017 18:29
@SparkQA
Copy link

SparkQA commented Jun 5, 2017

Test build #77753 has finished for PR 18176 at commit 6430dca.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sameeragarwal
Copy link
Member

cc @JoshRosen who had some concerns with shared threadpool objects

@JoshRosen
Copy link
Contributor

Discussion on this is happening on JIRA: https://issues.apache.org/jira/browse/SPARK-20952

@robert3005 robert3005 force-pushed the rk/inheritable-thread-local branch from 6430dca to 25e8408 Compare June 23, 2017 14:19
@SparkQA
Copy link

SparkQA commented Jun 23, 2017

Test build #78531 has finished for PR 18176 at commit 25e8408.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ash211
Copy link
Contributor

ash211 commented Jun 27, 2017

Jenkins this is ok to test

@SparkQA
Copy link

SparkQA commented Jun 27, 2017

Test build #78719 has finished for PR 18176 at commit 25e8408.

  • This patch fails PySpark pip packaging tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@robert3005 robert3005 changed the title [SPARK-20952] Make TaskContext an InheritableTheadLocal [SPARK-20952] ParquetFileFormat should forward TaskContext to its forkjoinpool Jul 29, 2017
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@robert3005 robert3005 closed this Feb 15, 2018
@robert3005 robert3005 deleted the rk/inheritable-thread-local branch February 15, 2018 14:56
rshkv added a commit to palantir/spark that referenced this pull request Feb 23, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
rshkv added a commit to palantir/spark that referenced this pull request Feb 25, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
rshkv added a commit to palantir/spark that referenced this pull request Feb 26, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
rshkv added a commit to palantir/spark that referenced this pull request Feb 26, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
jdcasale added a commit to palantir/spark that referenced this pull request Mar 3, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
rshkv added a commit to palantir/spark that referenced this pull request Mar 4, 2021
…kjoinpool

See ticket [1] and the PR [2] @robert3005 opened on apache/spark. It
seems upstream wasn't convinced by our use-case.

We need this change because Spark reads parquet footers on a different
thread. Without this change, that thread doesn't inherit the
thread-local that stores the TaskContext, meaning we don't have access
to the properties stored inside that TaskContext

Our internal filesystem needs those properties in the task context and
fails to read footers without.

[1] https://issues.apache.org/jira/browse/SPARK-20952
[2] apache#18176

Co-authored-by: Robert Kruszewski <[email protected]>
Co-authored-by: Josh Casale <[email protected]>
Co-authored-by: Will Raschkowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants