Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions python/pyspark/sql/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,15 @@ def foreachPartition(self, f):
"""
self.rdd.foreachPartition(f)

@since(2.0)
def refresh(self):
"""Refreshes the metadata and data cached in Spark for data associated with this DataFrame.

An example use case is to invalidate the file system metadata cached by Spark, when the
underlying files have been updated by an external process.
"""
self._jdf.refresh()

@since(1.3)
def cache(self):
""" Persists with the default storage level (C{MEMORY_ONLY}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,17 +462,15 @@ class SessionCatalog(
}
}

// TODO: It's strange that we have both refresh and invalidate here.

/**
* Refresh the cache entry for a metastore table, if any.
*/
def refreshTable(name: TableIdentifier): Unit = { /* no-op */ }

/**
* Invalidate the cache entry for a metastore table, if any.
*/
def invalidateTable(name: TableIdentifier): Unit = { /* no-op */ }
def refreshTable(name: TableIdentifier): Unit = {
// Go through temporary tables and invalidate them.
if (name.database.isEmpty) {
tempTables.get(name.table).foreach(_.refresh())
}
}

/**
* Drop all existing temporary tables.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ abstract class LogicalPlan extends QueryPlan[LogicalPlan] with Logging {
s"Reference '$name' is ambiguous, could be: $referenceNames.")
}
}

/**
* Invalidates any metadata cached in the plan recursively.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Refreshes" instead of "Invalidates"?

*/
def refresh(): Unit = children.foreach(_.refresh())
Copy link
Member

@gatorsmile gatorsmile Jun 30, 2016

Choose a reason for hiding this comment

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

use @tailrec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a tailrec?

Copy link
Member

Choose a reason for hiding this comment

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

You need to mark it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this function is not tail recursive.

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to avoid recursive implementation at best. It is too expensive for a large tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get it. Why would this be more expensive than any other recursive calls that happen in logical plans?

Copy link
Member

Choose a reason for hiding this comment

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

This is the comment I got when I using the recursive implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you point me to it? The entire TreeNode library I saw had a lot of recursive calls.

Copy link
Member

Choose a reason for hiding this comment

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

Let us wait for the comments from the Committers. They might be feeling OK for your scenario. Normally, recursive implementation is not encouraged.

Copy link
Contributor

Choose a reason for hiding this comment

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

children.foreach(_.refresh()) isn't tail recursive. Using a non-tail recursion here is probably fine since this isn't a critical path. As @petermaxlee said, we are already using recursion to handle TreeNodes everywhere with the assumption that such trees are relatively shallow. We do need to pay attention when using recursion in other places though.

}

/**
Expand Down
13 changes: 13 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2306,6 +2306,19 @@ class Dataset[T] private[sql](
*/
def distinct(): Dataset[T] = dropDuplicates()

/**
* Refreshes the metadata and data cached in Spark for data associated with this Dataset.
* An example use case is to invalidate the file system metadata cached by Spark, when the
* underlying files have been updated by an external process.
*
* @group action
* @since 2.0.0
*/
def refresh(): Unit = {
unpersist(false)
Copy link
Member

Choose a reason for hiding this comment

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

It will remove the cached data. This is different from what JIRA describes. CC @rxin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other refresh methods also remove cached data, so I thought this is better.

Copy link
Member

Choose a reason for hiding this comment

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

This new API has different behaviors from the refreshTable API and Refresh Table SQL statement. See the following code:

/**
* Refresh the cache entry for a table, if any. For Hive metastore table, the metadata
* is refreshed.
*
* @group cachemgmt
* @since 2.0.0
*/
override def refreshTable(tableName: String): Unit = {
val tableIdent = sparkSession.sessionState.sqlParser.parseTableIdentifier(tableName)
sessionCatalog.refreshTable(tableIdent)
// If this table is cached as an InMemoryRelation, drop the original
// cached version and make the new version cached lazily.
val logicalPlan = sparkSession.sessionState.catalog.lookupRelation(tableIdent)
// Use lookupCachedData directly since RefreshTable also takes databaseName.
val isCached = sparkSession.sharedState.cacheManager.lookupCachedData(logicalPlan).nonEmpty
if (isCached) {
// Create a data frame to represent the table.
// TODO: Use uncacheTable once it supports database name.
val df = Dataset.ofRows(sparkSession, logicalPlan)
// Uncache the logicalPlan.
sparkSession.sharedState.cacheManager.uncacheQuery(df, blocking = true)
// Cache it again.
sparkSession.sharedState.cacheManager.cacheQuery(df, Some(tableIdent.table))
}
}

IMO, if we using the word refresh, we have to make them consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ic - we can't unpersist.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can unpersist, but should persist it again immediately.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we can and should call unpersist, but we should also call persist()/cache() again so that the Dataset will be cached lazily again with correct data when it gets executed next time. I guess that's also what @gatorsmile meant.

logicalPlan.refresh()
}

/**
* Persist this Dataset with the default storage level (`MEMORY_AND_DISK`).
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ case class DropTableCommand(
} catch {
case NonFatal(e) => log.warn(e.toString, e)
}
catalog.invalidateTable(tableName)
catalog.refreshTable(tableName)
catalog.dropTable(tableName, ifExists)
}
Seq.empty[Row]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ case class AlterTableRenameCommand(
}
// Invalidate the table last, otherwise uncaching the table would load the logical plan
// back into the hive metastore cache
catalog.invalidateTable(oldName)
catalog.refreshTable(oldName)
catalog.renameTable(oldName, newName)
if (wasCached) {
sparkSession.catalog.cacheTable(newName.unquotedString)
Expand Down Expand Up @@ -373,7 +373,7 @@ case class TruncateTableCommand(
}
// After deleting the data, invalidate the table to make sure we don't keep around a stale
// file relation in the metastore cache.
spark.sessionState.invalidateTable(tableName.unquotedString)
spark.sessionState.refreshTable(tableName.unquotedString)
// Also try to drop the contents of the table from the columnar cache
try {
spark.sharedState.cacheManager.uncacheQuery(spark.table(tableName.quotedString))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,19 @@ class FileScanRDD(
currentFile = files.next()
logInfo(s"Reading File $currentFile")
InputFileNameHolder.setInputFileName(currentFile.filePath)
currentIterator = readFunction(currentFile)

try {
currentIterator = readFunction(currentFile)
} catch {
case e: java.io.FileNotFoundException =>
throw new java.io.FileNotFoundException(
e.getMessage + "\n" +
"It is possible the underlying files have been updated. " +
"You can explicitly invalidate the cache in Spark by " +
"running 'REFRESH TABLE tableName' command in SQL or " +
"by calling the refresh() function on a Dataset/DataFrame."
)
}
hasNext
} else {
currentFile = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,10 @@ case class LogicalRelation(
expectedOutputAttributes,
metastoreTableIdentifier).asInstanceOf[this.type]

override def refresh(): Unit = relation match {
case fs: HadoopFsRelation => fs.refresh()
Copy link
Member

Choose a reason for hiding this comment

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

How about the other leaf nodes? LogicalRelation is just one of them.

Copy link
Member

Choose a reason for hiding this comment

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

You have to document the reason why only LogicalRelation override this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? Other leaf nodes don't keep state, do they?

Copy link
Member

Choose a reason for hiding this comment

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

I know, but we need to write the comments for the code readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't agree on this one. LogicalRelation might not be the only one that needs to override this in the future. There can certainly be other logical plans in the future that keep some state and needs to implement refresh. The definition of "refresh" itself with a default implementation also means only plans that need to refresh anything should override it.

I'm going to update refresh in LogicalPlan to make this more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be refreshed?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we can invalidate the cache entry in cachedDataSourceTables

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, yeah, it is unable to be refreshed.

Copy link
Member

Choose a reason for hiding this comment

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

cachedDataSourceTables is only for the converted MetastoreRelation

Copy link
Member

Choose a reason for hiding this comment

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

When we call Dataset.refresh() on the true MetastoreRelation (non-converted Hive tables), we have to rebuild the plan, right?

case _ => // Do nothing.
}

override def simpleString: String = s"Relation[${Utils.truncatedString(output, ",")}] $relation"
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ private[sql] class SessionState(sparkSession: SparkSession) {

def executePlan(plan: LogicalPlan): QueryExecution = new QueryExecution(sparkSession, plan)

def invalidateTable(tableName: String): Unit = {
catalog.invalidateTable(sqlParser.parseTableIdentifier(tableName))
def refreshTable(tableName: String): Unit = {
Copy link
Member

Choose a reason for hiding this comment

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

To be honest, I still think invalidateTable is a right name. We are not doing refresh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just picked the one that was exposed to users (refresh in catalog and in sql).

catalog.refreshTable(sqlParser.parseTableIdentifier(tableName))
}

def addJar(path: String): Unit = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.sql

import java.io.File

import org.apache.spark.SparkException
import org.apache.spark.sql.test.SharedSQLContext

class MetadataCacheSuite extends QueryTest with SharedSQLContext {

/** Removes one data file in the given directory. */
private def deleteOneFileInDirectory(dir: File): Unit = {
assert(dir.isDirectory)
val oneFile = dir.listFiles().find { file =>
!file.getName.startsWith("_") && !file.getName.startsWith(".")
}
assert(oneFile.isDefined)
oneFile.foreach(_.delete())
}

test("Dataset.refresh()") {
withTempPath { (location: File) =>
// Create a Parquet directory
spark.range(start = 0, end = 100, step = 1, numPartitions = 3)
.write.parquet(location.getAbsolutePath)

// Read the directory in
val df = spark.read.parquet(location.getAbsolutePath)
assert(df.count() == 100)

// Delete a file
deleteOneFileInDirectory(location)

// Read it again and now we should see a FileNotFoundException
val e = intercept[SparkException] {
df.count()
}
assert(e.getMessage.contains("FileNotFoundException"))
assert(e.getMessage.contains("refresh()"))

// Refresh and we should be able to read it again.
df.refresh()
assert(df.count() > 0 && df.count() < 100)
}
}

test("temporary view refresh") {
withTempPath { (location: File) =>
// Create a Parquet directory
spark.range(start = 0, end = 100, step = 1, numPartitions = 3)
.write.parquet(location.getAbsolutePath)

// Read the directory in
spark.read.parquet(location.getAbsolutePath).createOrReplaceTempView("view_refresh")
assert(sql("select count(*) from view_refresh").first().getLong(0) == 100)

// Delete a file
deleteOneFileInDirectory(location)

// Read it again and now we should see a FileNotFoundException
val e = intercept[SparkException] {
sql("select count(*) from view_refresh").first()
}
assert(e.getMessage.contains("FileNotFoundException"))
assert(e.getMessage.contains("refresh()"))

// Refresh and we should be able to read it again.
spark.catalog.refreshTable("view_refresh")
val newCount = sql("select count(*) from view_refresh").first().getLong(0)
assert(newCount > 0 && newCount < 100)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,6 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log
}

def refreshTable(tableIdent: TableIdentifier): Unit = {
// refreshTable does not eagerly reload the cache. It just invalidate the cache.
// Next time when we use the table, it will be populated in the cache.
// Since we also cache ParquetRelations converted from Hive Parquet tables and
// adding converted ParquetRelations into the cache is not defined in the load function
// of the cache (instead, we add the cache entry in convertToParquetRelation),
// it is better at here to invalidate the cache to avoid confusing waring logs from the
// cache loader (e.g. cannot find data source provider, which is only defined for
// data source table.).
invalidateTable(tableIdent)
}

def invalidateTable(tableIdent: TableIdentifier): Unit = {
cachedDataSourceTables.invalidate(getQualifiedTableName(tableIdent))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,10 @@ private[sql] class HiveSessionCatalog(
val CreateTables: Rule[LogicalPlan] = metastoreCatalog.CreateTables

override def refreshTable(name: TableIdentifier): Unit = {
super.refreshTable(name)
metastoreCatalog.refreshTable(name)
}

override def invalidateTable(name: TableIdentifier): Unit = {
metastoreCatalog.invalidateTable(name)
}

def invalidateCache(): Unit = {
metastoreCatalog.cachedDataSourceTables.invalidateAll()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,13 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
sql("SELECT `c_!@(3)` FROM expectedJsonTable").collect().toSeq)

// Discard the cached relation.
sessionState.invalidateTable("jsonTable")
sessionState.refreshTable("jsonTable")

checkAnswer(
sql("SELECT * FROM jsonTable"),
sql("SELECT `c_!@(3)` FROM expectedJsonTable").collect().toSeq)

sessionState.invalidateTable("jsonTable")
sessionState.refreshTable("jsonTable")
val expectedSchema = StructType(StructField("c_!@(3)", IntegerType, true) :: Nil)

assert(expectedSchema === table("jsonTable").schema)
Expand Down Expand Up @@ -349,7 +349,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
""".stripMargin)

// Discard the cached relation.
sessionState.invalidateTable("ctasJsonTable")
sessionState.refreshTable("ctasJsonTable")

// Schema should not be changed.
assert(table("ctasJsonTable").schema === table("jsonTable").schema)
Expand Down Expand Up @@ -424,7 +424,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
sql("SELECT * FROM savedJsonTable tmp where tmp.a > 5"),
(6 to 10).map(i => Row(i, s"str$i")))

sessionState.invalidateTable("savedJsonTable")
sessionState.refreshTable("savedJsonTable")

checkAnswer(
sql("SELECT * FROM savedJsonTable where savedJsonTable.a < 5"),
Expand Down Expand Up @@ -710,7 +710,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
options = Map("path" -> tempDir.getCanonicalPath),
isExternal = false)

sessionState.invalidateTable("wide_schema")
sessionState.refreshTable("wide_schema")

val actualSchema = table("wide_schema").schema
assert(schema === actualSchema)
Expand Down Expand Up @@ -743,7 +743,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv

sharedState.externalCatalog.createTable("default", hiveTable, ignoreIfExists = false)

sessionState.invalidateTable(tableName)
sessionState.refreshTable(tableName)
val actualSchema = table(tableName).schema
assert(schema === actualSchema)

Expand All @@ -758,7 +758,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv

withTable(tableName) {
df.write.format("parquet").partitionBy("d", "b").saveAsTable(tableName)
sessionState.invalidateTable(tableName)
sessionState.refreshTable(tableName)
val metastoreTable = sharedState.externalCatalog.getTable("default", tableName)
val expectedPartitionColumns = StructType(df.schema("d") :: df.schema("b") :: Nil)

Expand Down Expand Up @@ -793,7 +793,7 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
.bucketBy(8, "d", "b")
.sortBy("c")
.saveAsTable(tableName)
sessionState.invalidateTable(tableName)
sessionState.refreshTable(tableName)
val metastoreTable = sharedState.externalCatalog.getTable("default", tableName)
val expectedBucketByColumns = StructType(df.schema("d") :: df.schema("b") :: Nil)
val expectedSortByColumns = StructType(df.schema("c") :: Nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ class ParquetMetastoreSuite extends ParquetPartitioningTest {
checkCached(tableIdentifier)
// For insert into non-partitioned table, we will do the conversion,
// so the converted test_insert_parquet should be cached.
sessionState.invalidateTable("test_insert_parquet")
sessionState.refreshTable("test_insert_parquet")
assert(sessionState.catalog.getCachedDataSourceTable(tableIdentifier) === null)
sql(
"""
Expand All @@ -475,7 +475,7 @@ class ParquetMetastoreSuite extends ParquetPartitioningTest {
sql("select * from test_insert_parquet"),
sql("select a, b from jt").collect())
// Invalidate the cache.
sessionState.invalidateTable("test_insert_parquet")
sessionState.refreshTable("test_insert_parquet")
assert(sessionState.catalog.getCachedDataSourceTable(tableIdentifier) === null)

// Create a partitioned table.
Expand Down Expand Up @@ -525,7 +525,7 @@ class ParquetMetastoreSuite extends ParquetPartitioningTest {
|select b, '2015-04-02', a FROM jt
""".stripMargin).collect())

sessionState.invalidateTable("test_parquet_partitioned_cache_test")
sessionState.refreshTable("test_parquet_partitioned_cache_test")
assert(sessionState.catalog.getCachedDataSourceTable(tableIdentifier) === null)

dropTables("test_insert_parquet", "test_parquet_partitioned_cache_test")
Expand Down