-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-5998] Speed up reads from bootstrapped tables in spark #8303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
27b8276
221af92
4bce566
7f9a12f
4a20074
498f23e
78befd9
5bca709
6a7ae70
9cda89b
732fbf0
c6908a1
3cfef7f
76394b7
e779563
3ad5ae5
e4144fb
2e63f7a
0ed2644
b8772a7
f361b40
27375ab
551c52d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ import org.apache.hudi.common.model.HoodieTableType.{COPY_ON_WRITE, MERGE_ON_REA | |
| import org.apache.hudi.common.table.timeline.HoodieInstant | ||
| import org.apache.hudi.common.table.{HoodieTableMetaClient, TableSchemaResolver} | ||
| import org.apache.hudi.common.util.ValidationUtils.checkState | ||
| import org.apache.hudi.config.HoodieBootstrapConfig | ||
| import org.apache.hudi.config.HoodieWriteConfig.WRITE_CONCURRENCY_MODE | ||
| import org.apache.hudi.exception.HoodieException | ||
| import org.apache.hudi.util.PathUtils | ||
|
|
@@ -100,7 +101,7 @@ class DefaultSource extends RelationProvider | |
| ) | ||
| } else { | ||
| Map() | ||
| }) ++ DataSourceOptionsHelper.parametersWithReadDefaults(optParams) | ||
| }) ++ DataSourceOptionsHelper.parametersWithReadDefaults(sqlContext.getAllConfs.filter(k => k._1.startsWith("hoodie.")) ++ optParams) | ||
|
|
||
| // Get the table base path | ||
| val tablePath = if (globPaths.nonEmpty) { | ||
|
|
@@ -261,7 +262,7 @@ object DefaultSource { | |
| new MergeOnReadIncrementalRelation(sqlContext, parameters, metaClient, userSchema) | ||
|
|
||
| case (_, _, true) => | ||
| new HoodieBootstrapRelation(sqlContext, userSchema, globPaths, metaClient, parameters) | ||
| resolveHoodieBootstrapRelation(sqlContext, globPaths, userSchema, metaClient, parameters) | ||
|
|
||
| case (_, _, _) => | ||
| throw new HoodieException(s"Invalid query type : $queryType for tableType: $tableType," + | ||
|
|
@@ -270,6 +271,21 @@ object DefaultSource { | |
| } | ||
| } | ||
|
|
||
| private def resolveHoodieBootstrapRelation(sqlContext: SQLContext, | ||
| globPaths: Seq[Path], | ||
| userSchema: Option[StructType], | ||
| metaClient: HoodieTableMetaClient, | ||
| parameters: Map[String, String]): BaseRelation = { | ||
| val enableFileIndex = HoodieSparkConfUtils.getConfigValue(parameters, sqlContext.sparkSession.sessionState.conf, | ||
| ENABLE_HOODIE_FILE_INDEX.key, ENABLE_HOODIE_FILE_INDEX.defaultValue.toString).toBoolean | ||
| if (!enableFileIndex || globPaths.nonEmpty || parameters.getOrElse(HoodieBootstrapConfig.DATA_QUERIES_ONLY.key(), "true") != "true") { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should do away with the config and rely on the condition here to decide whether or not to use the fast read path (which should be done by default). Wdyt?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to read the metadata columns you need to disable it. I found a few tests that use the metadata columns and I would assume that some users must
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get it. But, does it need to be inferred through a separate config? Can we not infer from the already available parameters?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to know at the point of creating the relation, so I don't think this can be done
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jonvex : Wouldn't this change cause user queries which includes hoodie metadata columns to fail ? Can't we just userschema being passed here to determine if there are any hoodie metadata columns being queried to determine appropriate next steps ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm, @jonvex : if you look at HoodieBootstrapRelation.composeRDD (the relation is being instantiated in below line), we segregate the skeleton schema and base file schema. Can we move the optimization logic inside that ? My main concern is this would break the existing functionality of bootstrap queries including hudi metafields failing unless user turn off the feature.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spark applies special optimizations to HadoopFsRelation so unless we contribute PRs to spark, this is the only way to do it as far as I can tell
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate what optimization are being done to HadoopFsRelation that causes 100% speed up ? I don't seem to find this information from the PR description.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://issues.apache.org/jira/browse/HUDI-3896 I am not sure if this is the only optimization, but it is one of them. The query plans for non bootstrapped and bootstrap tables look pretty much identical except non bootstrap says "FileScan parquet" when reading and bootstrap reading says "scan HoodieBootstrapRelation" I started by comparing time to run tpcds queries on boostrapped tables vs non bootstrapped. For a full bootstrap, the runtime ratio was 1.997 and for a metadata only bootstrap it was 1.638. I thought that was surprising that the full bootstrap was so slow, so I tried to replicate what was being done in BaseFileOnlyRelation in the first commit in this pr. We create a HoodieFileScanRDD instead of a HoodieBootstrapRDD. The ratio of tpcds runtime compared to reading from a non bootstrap table was 1.48 for a full bootstrap table, and 1.35 for a metadata only bootstrap. With the changes in this pr to leverage HadoopFsRelation the ratio was 1.12 for metadata only bootstrap, and 1.09 for full bootstrap.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jonvex : Can we make HoodieBootstrapRelation/HoodieBaseRelation extend HadoopFsRelation to get the behavior ? |
||
| HoodieBootstrapRelation(sqlContext, userSchema, globPaths, metaClient, parameters) | ||
| } else { | ||
| HoodieBootstrapRelation(sqlContext, userSchema, globPaths, metaClient, parameters + | ||
| (HoodieBootstrapRelation.USE_FAST_BOOTSTRAP_READ -> "true")).toHadoopFsRelation | ||
| } | ||
| } | ||
|
|
||
| private def resolveBaseFileOnlyRelation(sqlContext: SQLContext, | ||
| globPaths: Seq[Path], | ||
| userSchema: Option[StructType], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we can't set read configs in spark sql using the syntax like "set hoodie.bootstrap.data.queries.only=false". It only works for write configs. This was something we wanted to add anyways: https://issues.apache.org/jira/browse/HUDI-5361