You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "jonvex (via GitHub)" <gi...@apache.org> on 2024/03/21 16:13:07 UTC
[PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
jonvex opened a new pull request, #10904:
URL: https://github.com/apache/hudi/pull/10904
### Change Logs
The dag will trigger if iterator.isEmpty is called.
### Impact
Prevent early execution and unnecessary computation. Let spark optimize and manage memory better.
### Risk level (write none, low medium or high below)
medium
### Documentation Update
N/A
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10904:
URL: https://github.com/apache/hudi/pull/10904#issuecomment-2013067234
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22980",
"triggerID" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 43fb7bf1963b5d2a37074b4a20a0a8cd5f116778 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22980)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex closed pull request #10904: [HUDI-7525] prevent dag trigger in mappartitions if possible
URL: https://github.com/apache/hudi/pull/10904
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10904:
URL: https://github.com/apache/hudi/pull/10904#issuecomment-2012885083
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 43fb7bf1963b5d2a37074b4a20a0a8cd5f116778 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1534863672
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
will it yield an empty data frame here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "jonvex (via GitHub)" <gi...@apache.org>.
jonvex commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1536639905
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
if (records.isEmpty) Iterator.empty
So wouldn't calling records.map still be empty?
No one I've asked so far knows why we did this, but I am also hesitant to make this change as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1538453838
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
Let's test all the relevant code path involving `records.isEmpty` with empty Spark partitions on multiple Spark versions to make sure the removal of `#isEmpty` is safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1536712080
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
yeah, the gains for `records.isEmpty` is little, do we have a test for empty records here? If the test pass, I think we can make this change anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1534863672
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
will it yields an empty data frame here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10904:
URL: https://github.com/apache/hudi/pull/10904#issuecomment-2013136834
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22980",
"triggerID" : "43fb7bf1963b5d2a37074b4a20a0a8cd5f116778",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 43fb7bf1963b5d2a37074b4a20a0a8cd5f116778 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22980)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7525] prevent dag trigger in mappartitions if possible [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10904:
URL: https://github.com/apache/hudi/pull/10904#discussion_r1536712080
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/hudi/AvroConversionUtils.scala:
##########
@@ -98,13 +98,10 @@ object AvroConversionUtils {
*/
def createDataFrame(rdd: RDD[GenericRecord], schemaStr: String, ss: SparkSession): Dataset[Row] = {
ss.createDataFrame(rdd.mapPartitions { records =>
- if (records.isEmpty) Iterator.empty
- else {
- val schema = new Schema.Parser().parse(schemaStr)
- val dataType = convertAvroSchemaToStructType(schema)
- val converter = createConverterToRow(schema, dataType)
- records.map { r => converter(r) }
- }
+ val schema = new Schema.Parser().parse(schemaStr)
Review Comment:
yeah, the gains for `records.isEmpty` is little, do we have a test for empty records here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org