You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2023/01/20 02:46:38 UTC
[GitHub] [hudi] wzx140 opened a new pull request, #7714: [HUDI-5549] SparkRecordManager support avro data block
wzx140 opened a new pull request, #7714:
URL: https://github.com/apache/hudi/pull/7714
### Change Logs
SparkRecordManager support avro data block
### Impact
Not change public API
### Risk level (write none, low medium or high below)
None
### Documentation Update
SparkRecordManager support avro data block
### 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
[GitHub] [hudi] hudi-bot commented on pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7714:
URL: https://github.com/apache/hudi/pull/7714#issuecomment-1397872078
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=14479",
"triggerID" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 462a4041585cfeb6c16354bbc8b964bdd08ed301 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=14479)
<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
[GitHub] [hudi] bvaradar commented on pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by "bvaradar (via GitHub)" <gi...@apache.org>.
bvaradar commented on PR #7714:
URL: https://github.com/apache/hudi/pull/7714#issuecomment-1497024107
@wzx140 : Can you rebase and resolve conflicts. I will take a look at this PR.
--
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
[GitHub] [hudi] hudi-bot commented on pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7714:
URL: https://github.com/apache/hudi/pull/7714#issuecomment-1398082322
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=14479",
"triggerID" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 462a4041585cfeb6c16354bbc8b964bdd08ed301 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=14479)
<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
[GitHub] [hudi] wzx140 commented on a diff in pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by GitBox <gi...@apache.org>.
wzx140 commented on code in PR #7714:
URL: https://github.com/apache/hudi/pull/7714#discussion_r1082263118
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java:
##########
@@ -192,16 +198,25 @@ public boolean hasNext() {
}
@Override
- public IndexedRecord next() {
+ public HoodieRecord next() {
try {
int recordLength = this.dis.readInt();
BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(this.content, this.dis.getNumberOfBytesRead(),
recordLength, this.decoderCache.get());
this.decoderCache.set(decoder);
- IndexedRecord record = this.reader.read(null, decoder);
+ IndexedRecord indexedRecord = this.reader.read(null, decoder);
this.dis.skipBytes(recordLength);
this.readRecords++;
- return record;
+ if (recordType == HoodieRecordType.SPARK) {
Review Comment:
Do you mean that we should move the logic of reading avro data into HoodieAvroReader and HoodieSparkAvroReader? And we create these readers from HoodieFileReaderFactory. Just like usage of FileReader in HoodieParquetDataBlock.
--
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
[GitHub] [hudi] hudi-bot commented on pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7714:
URL: https://github.com/apache/hudi/pull/7714#issuecomment-1397865891
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "462a4041585cfeb6c16354bbc8b964bdd08ed301",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 462a4041585cfeb6c16354bbc8b964bdd08ed301 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
[GitHub] [hudi] alexeykudinkin commented on a diff in pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by GitBox <gi...@apache.org>.
alexeykudinkin commented on code in PR #7714:
URL: https://github.com/apache/hudi/pull/7714#discussion_r1082080586
##########
hudi-client/hudi-spark-client/src/main/scala/org/apache/spark/sql/HoodieInternalRowUtils.scala:
##########
@@ -210,6 +220,26 @@ object HoodieInternalRowUtils {
}
}
+ def getCachedAvroSerializer(schema: Schema): HoodieAvroSerializer = {
+ val avroSerializerMap = avroSerializerMapThreadLocal.get()
+ if (!avroSerializerMap.containsKey(schema)) {
Review Comment:
Lookups w/ Avro schema as keys are costly -- let's minimize number of lookups we need to make:
- We can use `getOrElseUpdate` to get/update in one lookup
- We can store pair (StructType, SerDe) to avoid the need to do 2 lookups
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java:
##########
@@ -192,16 +198,25 @@ public boolean hasNext() {
}
@Override
- public IndexedRecord next() {
+ public HoodieRecord next() {
try {
int recordLength = this.dis.readInt();
BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(this.content, this.dis.getNumberOfBytesRead(),
recordLength, this.decoderCache.get());
this.decoderCache.set(decoder);
- IndexedRecord record = this.reader.read(null, decoder);
+ IndexedRecord indexedRecord = this.reader.read(null, decoder);
this.dis.skipBytes(recordLength);
this.readRecords++;
- return record;
+ if (recordType == HoodieRecordType.SPARK) {
Review Comment:
We should hide this w/in the reader
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java:
##########
@@ -261,7 +281,16 @@ public static HoodieAvroDataBlock getBlock(byte[] content, Schema readerSchema,
int recordLength = dis.readInt();
Decoder decoder = DecoderFactory.get().binaryDecoder(content, dis.getNumberOfBytesRead(), recordLength, null);
IndexedRecord record = reader.read(null, decoder);
- records.add(new HoodieAvroIndexedRecord(record));
+ if (recordType == HoodieRecordType.SPARK) {
Review Comment:
Same comment regarding reader
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -329,8 +329,12 @@ object HoodieSparkSqlWriter {
DataSourceUtils.createHoodieClient(jsc, dataFileSchemaStr, path, tblName, mapAsJavaMap(finalOpts))
}.asInstanceOf[SparkRDDWriteClient[HoodieRecordPayload[Nothing]]]
val writeConfig = client.getConfig
- if (writeConfig.getRecordMerger.getRecordType == HoodieRecordType.SPARK && tableType == HoodieTableType.MERGE_ON_READ && writeConfig.getLogDataBlockFormat.orElse(HoodieLogBlockType.AVRO_DATA_BLOCK) != HoodieLogBlockType.PARQUET_DATA_BLOCK) {
- throw new UnsupportedOperationException(s"${writeConfig.getRecordMerger.getClass.getName} only support parquet log.")
+ val logBlockType = writeConfig.getLogDataBlockFormat.orElse(HoodieLogBlockType.AVRO_DATA_BLOCK)
Review Comment:
We should leverage `pickLogDataBlockFormat` from `AppendHandle`
- We should move it to `WriteConfig`
- Make it accept `BaseFileFormat`
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -329,8 +329,12 @@ object HoodieSparkSqlWriter {
DataSourceUtils.createHoodieClient(jsc, dataFileSchemaStr, path, tblName, mapAsJavaMap(finalOpts))
}.asInstanceOf[SparkRDDWriteClient[HoodieRecordPayload[Nothing]]]
val writeConfig = client.getConfig
- if (writeConfig.getRecordMerger.getRecordType == HoodieRecordType.SPARK && tableType == HoodieTableType.MERGE_ON_READ && writeConfig.getLogDataBlockFormat.orElse(HoodieLogBlockType.AVRO_DATA_BLOCK) != HoodieLogBlockType.PARQUET_DATA_BLOCK) {
- throw new UnsupportedOperationException(s"${writeConfig.getRecordMerger.getClass.getName} only support parquet log.")
+ val logBlockType = writeConfig.getLogDataBlockFormat.orElse(HoodieLogBlockType.AVRO_DATA_BLOCK)
+ if (writeConfig.getRecordMerger.getRecordType == HoodieRecordType.SPARK
Review Comment:
Let's extract this to a standalone method `validateFileFormatsConfig`
--
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
[GitHub] [hudi] alexeykudinkin commented on a diff in pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
Posted by "alexeykudinkin (via GitHub)" <gi...@apache.org>.
alexeykudinkin commented on code in PR #7714:
URL: https://github.com/apache/hudi/pull/7714#discussion_r1083233962
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java:
##########
@@ -192,16 +198,25 @@ public boolean hasNext() {
}
@Override
- public IndexedRecord next() {
+ public HoodieRecord next() {
try {
int recordLength = this.dis.readInt();
BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(this.content, this.dis.getNumberOfBytesRead(),
recordLength, this.decoderCache.get());
this.decoderCache.set(decoder);
- IndexedRecord record = this.reader.read(null, decoder);
+ IndexedRecord indexedRecord = this.reader.read(null, decoder);
this.dis.skipBytes(recordLength);
this.readRecords++;
- return record;
+ if (recordType == HoodieRecordType.SPARK) {
Review Comment:
Correct
--
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-5549] SparkRecordManager support avro data block [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua closed pull request #7714: [HUDI-5549] SparkRecordManager support avro data block
URL: https://github.com/apache/hudi/pull/7714
--
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