yihua commented on code in PR #12826:
URL: https://github.com/apache/hudi/pull/12826#discussion_r1975964543
##########
hudi-common/src/main/java/org/apache/hudi/common/util/InternalSchemaCache.java:
##########
@@ -197,15 +196,14 @@ public static InternalSchema
getInternalSchemaByVersionId(long versionId, String
.findFirst().map(f -> new StoragePath(hoodieMetaPath, f)).orElse(null);
if (candidateCommitFile != null) {
try {
- byte[] data;
+ HoodieCommitMetadata metadata;
try (InputStream is = storage.open(candidateCommitFile)) {
- data = FileIOUtils.readAsByteArray(is);
+ metadata =
commitMetadataSerDe.deserialize(instantGenerator.createNewInstant(
+ new StoragePathInfo(candidateCommitFile, -1, false, (short)
0, 0L, 0L)),
+ is, () -> true, HoodieCommitMetadata.class);
Review Comment:
Should this be `false`, i.e., non-empty file?
```suggestion
is, () -> false, HoodieCommitMetadata.class);
```
##########
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java:
##########
@@ -182,7 +183,7 @@ public void testCommitMetadataSerde() throws Exception {
byte[] v1Bytes = v1SerDe.serialize(commitMetadata1).get();
System.out.println(new String(v1Bytes));
org.apache.hudi.common.model.HoodieCommitMetadata commitMetadata2 =
- COMMIT_METADATA_SER_DE.deserialize(legacyInstant, v1Bytes,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
+ COMMIT_METADATA_SER_DE.deserialize(legacyInstant, new
ByteArrayInputStream(v1Bytes), () -> true,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
Review Comment:
```suggestion
COMMIT_METADATA_SER_DE.deserialize(legacyInstant, new
ByteArrayInputStream(v1Bytes), () -> false,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
```
##########
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java:
##########
@@ -168,7 +169,7 @@ public void testCommitMetadataSerde() throws Exception {
HoodieInstant instant =
INSTANT_GENERATOR.createNewInstant(HoodieInstant.State.COMPLETED, "commit",
"1");
org.apache.hudi.common.model.HoodieCommitMetadata commitMetadata1 =
COMMIT_METADATA_SER_DE.deserialize(instant,
- serializedCommitMetadata,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
+ new ByteArrayInputStream(serializedCommitMetadata), () -> true,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
Review Comment:
Similar here
```suggestion
new ByteArrayInputStream(serializedCommitMetadata), () -> false,
org.apache.hudi.common.model.HoodieCommitMetadata.class);
```
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1093,8 +1092,7 @@ private static void
reAddLogFilesFromRollbackPlan(HoodieTableMetaClient dataTabl
HoodieInstant rollbackInstant =
factory.createNewInstant(HoodieInstant.State.REQUESTED,
HoodieTimeline.ROLLBACK_ACTION, instantTime);
HoodieInstant requested =
factory.getRollbackRequestedInstant(rollbackInstant);
try {
- HoodieRollbackPlan rollbackPlan =
TimelineMetadataUtils.deserializeAvroMetadata(
-
dataTableMetaClient.getActiveTimeline().readRollbackInfoAsBytes(requested).get(),
HoodieRollbackPlan.class);
+ HoodieRollbackPlan rollbackPlan =
dataTableMetaClient.getActiveTimeline().loadInstantContent(requested,
HoodieRollbackPlan.class);
Review Comment:
`readRollbackPlan`
##########
hudi-common/src/main/java/org/apache/hudi/common/util/InternalSchemaCache.java:
##########
@@ -135,20 +135,19 @@ private static Option<InternalSchema>
getSchemaByReadingCommitFile(long versionI
/**
* Get internalSchema and avroSchema for compaction/cluster operation.
*
- * @param metaClient current hoodie metaClient
+ * @param metaClient current hoodie metaClient
* @param compactionAndClusteringInstant first instant before current
compaction/cluster instant
* @return (internalSchemaStrOpt, avroSchemaStrOpt) a pair of
InternalSchema/avroSchema
*/
public static Pair<Option<String>, Option<String>>
getInternalSchemaAndAvroSchemaForClusteringAndCompaction(HoodieTableMetaClient
metaClient, String compactionAndClusteringInstant) {
// try to load internalSchema to support Schema Evolution
HoodieTimeline timelineBeforeCurrentCompaction =
metaClient.getCommitsAndCompactionTimeline().findInstantsBefore(compactionAndClusteringInstant).filterCompletedInstants();
- Option<HoodieInstant> lastInstantBeforeCurrentCompaction =
timelineBeforeCurrentCompaction.lastInstant();
+ Option<HoodieInstant> lastInstantBeforeCurrentCompaction =
timelineBeforeCurrentCompaction.lastInstant();
if (lastInstantBeforeCurrentCompaction.isPresent()) {
// try to find internalSchema
- byte[] data =
timelineBeforeCurrentCompaction.getInstantDetails(lastInstantBeforeCurrentCompaction.get()).get();
HoodieCommitMetadata metadata;
try {
- metadata =
metaClient.getCommitMetadataSerDe().deserialize(lastInstantBeforeCurrentCompaction.get(),
data, HoodieCommitMetadata.class);
+ metadata =
timelineBeforeCurrentCompaction.loadInstantContent(lastInstantBeforeCurrentCompaction.get(),
HoodieCommitMetadata.class);
Review Comment:
Similar here on using `readCommitMetadata`. Let's make sure there is no
caller of `InstantContent(instant, clazz)` outside `HoodieTimeline` class.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
