Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 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
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ case class DataSource(
lazy val providingClass: Class[_] = DataSource.lookupDataSource(className)
lazy val sourceInfo: SourceInfo = sourceSchema()
private val caseInsensitiveOptions = CaseInsensitiveMap(options)

private lazy val fileStatusCache = FileStatusCache.getOrCreate(sparkSession)
Copy link
Contributor

@cloud-fan cloud-fan Mar 2, 2017

Choose a reason for hiding this comment

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

what's the lifetime of this cache?

/**
* Get the schema of the given FileFormat, if provided by `userSpecifiedSchema`, or try to infer
* it. In the read path, only managed tables by Hive provide the partition columns properly when
Expand Down Expand Up @@ -122,7 +122,7 @@ case class DataSource(
val qualified = hdfsPath.makeQualified(fs.getUri, fs.getWorkingDirectory)
SparkHadoopUtil.get.globPathIfNecessary(qualified)
}.toArray
new InMemoryFileIndex(sparkSession, globbedPaths, options, None)
new InMemoryFileIndex(sparkSession, globbedPaths, options, None, fileStatusCache)
Copy link
Member

Choose a reason for hiding this comment

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

This also impacts the streaming code path. If it is fine to streaming, the code changes look good to me.

Copy link
Contributor Author

@windpiger windpiger Mar 2, 2017

Choose a reason for hiding this comment

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

I have make it local only in the no streaming FileFormat match case~

}
val partitionSchema = if (partitionColumns.isEmpty) {
// Try to infer partitioning, because no DataSource in the read path provides the partitioning
Expand Down Expand Up @@ -364,7 +364,12 @@ case class DataSource(
catalogTable.get,
catalogTable.get.stats.map(_.sizeInBytes.toLong).getOrElse(defaultTableSize))
} else {
new InMemoryFileIndex(sparkSession, globbedPaths, options, Some(partitionSchema))
new InMemoryFileIndex(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to create file status cache as a local variable, pass it to getOrInferFileFormatSchema, then use it here. It's much easier to reason about the lifetime of this cache by this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I think it is more reasonable~ thanks~

sparkSession,
globbedPaths,
options,
Some(partitionSchema),
fileStatusCache)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: indent issue

Copy link
Member

Choose a reason for hiding this comment

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

          new InMemoryFileIndex(
            sparkSession, globbedPaths, options, Some(partitionSchema), fileStatusCache)

This is also valid

}

HadoopFsRelation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.util.DateTimeUtils
import org.apache.spark.sql.execution.datasources.DataSource
import org.apache.spark.sql.test.SharedSQLContext

class ResolvedDataSourceSuite extends SparkFunSuite {
class ResolvedDataSourceSuite extends SparkFunSuite with SharedSQLContext {
private def getProvidingClass(name: String): Class[_] =
DataSource(
sparkSession = null,
sparkSession = spark,
className = name,
options = Map("timeZone" -> DateTimeUtils.defaultTimeZone().getID)).providingClass

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,4 +411,15 @@ class PartitionedTablePerfStatsSuite
}
}
}

test("resolveRelation for a FileFormat DataSource without userSchema scan filesystem only once") {
withTempDir { dir =>
import spark.implicits._
Seq(1).toDF("a").write.mode("overwrite").save(dir.getAbsolutePath)
HiveCatalogMetrics.reset()
spark.read.parquet(dir.getAbsolutePath)
assert(HiveCatalogMetrics.METRIC_FILES_DISCOVERED.getCount() == 1)
assert(HiveCatalogMetrics.METRIC_FILE_CACHE_HITS.getCount() == 1)
}
}
}