You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2017/07/23 04:26:09 UTC
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/18717
[SPARK-21510] [SQL] Add isMaterialized() and eager persist() to Dataset APIs
### What changes were proposed in this pull request?
Currently, when using Spark, the beginners do not realize our persist API is lazy. They do not know what is the most efficient way to materialize it. Sometimes, they just use collect(), which is very expensive when the data set is big. This PR is to add a simple API to enable users to do eager persist.
In addition, we also need another Dataset API to verify whether the Dataset has been cached and materialized.
### How was this patch tested?
Added two test cases and simplify the existing test cases by using the newly added API.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark eagerPersist
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18717.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18717
----
commit 7f82e38f8edca611cdf55266ff07ac4523799603
Author: gatorsmile <ga...@gmail.com>
Date: 2017-07-23T00:17:26Z
fix
commit 80e7123c3905c44776233e0a56a8d8315f1313a0
Author: gatorsmile <ga...@gmail.com>
Date: 2017-07-23T04:17:56Z
fix
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80887/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128923836
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
--- End diff --
We already have a few `isCached` APIs, which mean it is cached but it might not be materialized.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128914984
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
+ queryExecution.sparkPlan match {
+ case i: InMemoryTableScanExec =>
+ val blockManager = sparkSession.sparkContext.env.blockManager
+
+ val rdd = i.relation.cachedColumnBuffers
+ val blockIDs = rdd.partitions.indices.map(index => RDDBlockId(rdd.id, index))
+
+ var foundNonexistentBlocks = false
+ blockIDs.foreach { bid =>
+ if (blockManager.get(bid).isEmpty) {
+ foundNonexistentBlocks = true
+ } else {
+ blockManager.releaseLock(bid)
--- End diff --
Do you not need to release the block in all cases? maybe in try-finally? I don't know the BlockManager well though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79894 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79894/testReport)** for PR 18717 at commit [`9077258`](https://github.com/apache/spark/commit/9077258af86d7403e28723d9d6d457bd7bf1cc76).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80894/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79875/testReport)** for PR 18717 at commit [`80e7123`](https://github.com/apache/spark/commit/80e7123c3905c44776233e0a56a8d8315f1313a0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by onzo-mateuszzakarczemny <gi...@git.apache.org>.
Github user onzo-mateuszzakarczemny commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r197475963
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2759,6 +2760,34 @@ class Dataset[T] private[sql](
this
}
+ /**
+ * Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
+ persist()
+ if (eager) queryExecution.toRdd.foreachPartition(_ => {})
+ this
+ }
+
+ /**
+ * Persist this Dataset with the given storage level.
+ * @param eager If true, persist the Dataset eagerly.
+ * @param newLevel One of: `MEMORY_ONLY`, `MEMORY_AND_DISK`, `MEMORY_ONLY_SER`,
+ * `MEMORY_AND_DISK_SER`, `DISK_ONLY`, `MEMORY_ONLY_2`,
+ * `MEMORY_AND_DISK_2`, etc.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean, newLevel: StorageLevel): this.type = {
+ persist(newLevel)
+ if (eager) queryExecution.toRdd.foreachPartition(_ => {})
--- End diff --
I think it's worth to check if `.foreachPartition(_ => {})` always works. I tested that on plain RDD and it didn't trigger evaluation. I had to replace that construct with: `rdd.foreachPartition(_.foreach(_ => {}))`
However, maybe it works differently if you operate on `queryExecution`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128942266
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetCacheSuite.scala ---
@@ -25,9 +25,22 @@ import org.apache.spark.storage.StorageLevel
class DatasetCacheSuite extends QueryTest with SharedSQLContext {
import testImplicits._
- // Clear all persistent datasets after each test
override def afterEach(): Unit = {
- spark.sharedState.cacheManager.clearCache()
+ try {
+ // Clear all persistent datasets after each test
+ spark.sharedState.cacheManager.clearCache()
+ } finally {
+ super.afterEach()
+ }
+ }
+
+ test("eager persist") {
+ val ds = Seq("1", "2").toDF()
+ ds.persist(eager = false)
+ assert(!ds.isMaterialized())
+ ds.persist(eager = true)
+ ds.collect()
--- End diff --
To test if it is eagerly persisted, why we need a collect here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80888 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80888/testReport)** for PR 18717 at commit [`1589a81`](https://github.com/apache/spark/commit/1589a818294b917dccaa835c05924c8e021a5947).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80902/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79875/testReport)** for PR 18717 at commit [`80e7123`](https://github.com/apache/spark/commit/80e7123c3905c44776233e0a56a8d8315f1313a0).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128923156
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
+ queryExecution.sparkPlan match {
+ case i: InMemoryTableScanExec =>
+ val blockManager = sparkSession.sparkContext.env.blockManager
+
+ val rdd = i.relation.cachedColumnBuffers
+ val blockIDs = rdd.partitions.indices.map(index => RDDBlockId(rdd.id, index))
+
+ var foundNonexistentBlocks = false
+ blockIDs.foreach { bid =>
+ if (blockManager.get(bid).isEmpty) {
+ foundNonexistentBlocks = true
+ } else {
+ blockManager.releaseLock(bid)
--- End diff --
If so, we will hit an exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #89907 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89907/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128923128
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
--- End diff --
`isPersisted ` is like `isCached`. It is different from `isMaterialized`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128914998
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2706,6 +2707,18 @@ class Dataset[T] private[sql](
/**
* Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
+ persist()
+ if (eager) queryExecution.toRdd.count()
--- End diff --
count() is not efficient though. You can mapPartitions with a no-op function instead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79894/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79894 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79894/testReport)** for PR 18717 at commit [`9077258`](https://github.com/apache/spark/commit/9077258af86d7403e28723d9d6d457bd7bf1cc76).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80888/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #89907 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89907/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
* This patch **fails PySpark unit tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80902 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80902/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by Matzz <gi...@git.apache.org>.
Github user Matzz commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r197476588
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2759,6 +2760,34 @@ class Dataset[T] private[sql](
this
}
+ /**
+ * Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
+ persist()
+ if (eager) queryExecution.toRdd.foreachPartition(_ => {})
+ this
+ }
+
+ /**
+ * Persist this Dataset with the given storage level.
+ * @param eager If true, persist the Dataset eagerly.
+ * @param newLevel One of: `MEMORY_ONLY`, `MEMORY_AND_DISK`, `MEMORY_ONLY_SER`,
+ * `MEMORY_AND_DISK_SER`, `DISK_ONLY`, `MEMORY_ONLY_2`,
+ * `MEMORY_AND_DISK_2`, etc.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean, newLevel: StorageLevel): this.type = {
+ persist(newLevel)
+ if (eager) queryExecution.toRdd.foreachPartition(_ => {})
--- End diff --
I think it's worth to check if `.foreachPartition(_ => {})` always works. I tested that on plain RDD and it didn't trigger evaluation. I had to replace that construct with: `rdd.foreachPartition(_.foreach(_ => {}))`
However, maybe it works differently if you operate on queryExecution
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128923757
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2706,6 +2707,18 @@ class Dataset[T] private[sql](
/**
* Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
+ persist()
+ if (eager) queryExecution.toRdd.count()
--- End diff --
I see. I think you are saying `foreachPartition`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/18717
The target of this ticket is 2.4?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79890/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128914959
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
--- End diff --
isPersisted?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128919307
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,34 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
+ queryExecution.sparkPlan match {
+ case i: InMemoryTableScanExec =>
+ val blockManager = sparkSession.sparkContext.env.blockManager
+
+ val rdd = i.relation.cachedColumnBuffers
+ val blockIDs = rdd.partitions.indices.map(index => RDDBlockId(rdd.id, index))
+
+ var foundNonexistentBlocks = false
+ blockIDs.foreach { bid =>
+ if (blockManager.get(bid).isEmpty) {
+ foundNonexistentBlocks = true
--- End diff --
Can we immediately execute `return false` here without `foundNonexistentBlocks`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/18717
@gatorsmile what will we need to go forward?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r129185097
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala ---
@@ -226,6 +216,21 @@ abstract class QueryTest extends PlanTest {
}
/**
+ * Asserts that a given [[Dataset]] has been materialized.
+ */
+ def assertMaterialized(query: Dataset[_]): Unit = {
+ assert(query.isMaterialized(),
+ "Eagerly cached cached Dataset should have already been materialized")
--- End diff --
`cached cached` -> `cached`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128944222
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,26 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
--- End diff --
What is the use scenarios for this API? RDD also doesn't have the API to check it's materialized. I think in practice, we don't need to check if an Dataset/RDD is materialized, except for test purpose.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80902 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80902/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80888 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80888/testReport)** for PR 18717 at commit [`1589a81`](https://github.com/apache/spark/commit/1589a818294b917dccaa835c05924c8e021a5947).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18717
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r132783501
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,26 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
+ queryExecution.sparkPlan match {
+ case i: InMemoryTableScanExec =>
+ val blockManager = sparkSession.sparkContext.env.blockManager
+ val rdd = i.relation.cachedColumnBuffers
+ val blockIDs = rdd.partitions.indices.map(index => RDDBlockId(rdd.id, index))
+ blockIDs.foreach { bid =>
+ if (blockManager.get(bid).nonEmpty) blockManager.releaseLock(bid) else return false
--- End diff --
We can do it in a cheaper way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r132783610
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,26 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
--- End diff --
This PR will also do it for RDD API
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128927009
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/cache.scala ---
@@ -41,7 +41,7 @@ case class CacheTableCommand(
if (!isLazy) {
// Performs eager caching
- sparkSession.table(tableIdent).count()
+ sparkSession.table(tableIdent).persist(eager = true)
--- End diff --
Our SQL command `CACHE [LAZY] TABLE` has an eager/lazy interface.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80894 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80894/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79890 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79890/testReport)** for PR 18717 at commit [`e63efab`](https://github.com/apache/spark/commit/e63efabe5ac48133aab877914adaaadd1fc82ed7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18717
Time : )
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79875/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89907/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128923428
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2706,6 +2707,18 @@ class Dataset[T] private[sql](
/**
* Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
--- End diff --
Here, this PR is trying to introduce high level helper APIs, which are useful to end users based on the feedback I got. The storage level is not used very commonly. If needed, we can add another API in the future?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128943409
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2739,6 +2752,26 @@ class Dataset[T] private[sql](
}
/**
+ * Returns true when the Dataset is cached and materialized.
+ *
+ * @group basic
+ * @since 2.3.0
+ */
+ def isMaterialized(): Boolean = {
+ queryExecution.sparkPlan match {
+ case i: InMemoryTableScanExec =>
+ val blockManager = sparkSession.sparkContext.env.blockManager
+ val rdd = i.relation.cachedColumnBuffers
+ val blockIDs = rdd.partitions.indices.map(index => RDDBlockId(rdd.id, index))
+ blockIDs.foreach { bid =>
+ if (blockManager.get(bid).nonEmpty) blockManager.releaseLock(bid) else return false
--- End diff --
I think this won't work for remote blocks. `BlockManager.get` doesn't acquire any locks for remote blocks. Once you release lock on them, you will get exception.
Btw, for remote blocks, it fetches the blocks back, so it's costly for this purpose.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18717: [SPARK-21510] [SQL] Add isMaterialized() and eage...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/18717#discussion_r128914958
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2706,6 +2707,18 @@ class Dataset[T] private[sql](
/**
* Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
+ * @param eager If true, persist the Dataset eagerly.
+ * @group basic
+ * @since 2.3.0
+ */
+ def persist(eager: Boolean): this.type = {
--- End diff --
This varies from the RDD API then, which lets you control the persistence level, but doesn't expose this argument.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #80894 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80894/testReport)** for PR 18717 at commit [`31bf797`](https://github.com/apache/spark/commit/31bf79742a219ca13cca2a6774da783b061864d1).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18717
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18717: [SPARK-21510] [SQL] Add isMaterialized() and eager persi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18717
**[Test build #79890 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79890/testReport)** for PR 18717 at commit [`e63efab`](https://github.com/apache/spark/commit/e63efabe5ac48133aab877914adaaadd1fc82ed7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org