This is an automated email from the ASF dual-hosted git repository.
bhasudha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new 442261014f39 refactor(common): utils consolidation sweep remnants
(#19284)
442261014f39 is described below
commit 442261014f396a24cdbddb5df2b7e80e7bcce991
Author: vinoth chandar <[email protected]>
AuthorDate: Thu Jul 16 12:09:36 2026 -0700
refactor(common): utils consolidation sweep remnants (#19284)
Follow-ups from the #19212 review, all behavior-preserving:
- SecondaryIndexKeyUtils: rename
getUnescapedSecondaryKey[Prefix]FromSecondaryIndexKey
to getEscapedSecondaryKey[Prefix]FromSecondaryIndexKey -- both return the
still-encoded segment (the sibling that actually unescapes calls them), so
the old names said the opposite of what the methods do.
- Delete HoodieHiveUtils#getNthParent, a byte-identical copy of
TablePathUtils' private walker; make the latter public and point the one
remaining caller (HoodieROTablePathFilter) at it.
- Fold CommonClientUtils#generateWriteToken into FSUtils as a
makeWriteToken(TaskContextSupplier) overload next to its 3-arg sibling;
migrate the single caller (RollbackHelperV1).
Also audited all HoodieAvroUtils#unwrapNullable call sites for multi-member
union exposure (follow-up from the nullable-union dedup): no reachable
mis-resolution -- variant-shredding sites operate on internally-constructed
schemas, and the one arbitrary-schema site (projectFields) either discards
the unwrapped result or immediately validates it is a RECORD and throws.
---
.../hudi/table/action/rollback/RollbackHelperV1.java | 5 ++---
.../java/org/apache/hudi/util/CommonClientUtils.java | 16 ----------------
.../org/apache/hudi/utils/TestCommonClientUtils.java | 11 -----------
.../main/java/org/apache/hudi/common/fs/FSUtils.java | 18 ++++++++++++++++++
.../org/apache/hudi/common/util/TablePathUtils.java | 5 ++++-
.../apache/hudi/metadata/HoodieTableMetadataUtil.java | 2 +-
.../apache/hudi/metadata/SecondaryIndexKeyUtils.java | 6 +++---
.../hudi/metadata/TestSecondaryIndexKeyUtils.java | 12 ++++++------
.../java/org/apache/hudi/common/fs/TestFSUtils.java | 14 ++++++++++++++
.../apache/hudi/hadoop/HoodieROTablePathFilter.java | 5 +++--
.../org/apache/hudi/hadoop/utils/HoodieHiveUtils.java | 16 ----------------
11 files changed, 51 insertions(+), 59 deletions(-)
diff --git
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/RollbackHelperV1.java
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/RollbackHelperV1.java
index 3a3fa3c08c9a..d5b81f350f1a 100644
---
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/RollbackHelperV1.java
+++
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/RollbackHelperV1.java
@@ -52,7 +52,6 @@ import org.apache.hudi.table.HoodieTable;
import org.apache.hudi.table.marker.AppendMarkerHandler;
import org.apache.hudi.table.marker.WriteMarkers;
import org.apache.hudi.table.marker.WriteMarkersFactory;
-import org.apache.hudi.util.CommonClientUtils;
import lombok.extern.slf4j.Slf4j;
@@ -320,7 +319,7 @@ public class RollbackHelperV1 extends RollbackHelper {
HoodieLogFormatWriter.HoodieLogFormatWriterBuilder writerBuilder =
HoodieLogFormatWriter.builder()
.withParentPath(FSUtils.constructAbsolutePath(metaClient.getBasePath(),
partitionPath))
.withLogFileId(fileId)
-
.withLogWriteToken(CommonClientUtils.generateWriteToken(taskContextSupplier))
+ .withLogWriteToken(FSUtils.makeWriteToken(taskContextSupplier))
.withInstantTime(tableVersion.greaterThanOrEquals(HoodieTableVersion.EIGHT)
? instantToRollback.requestedTime() :
rollbackRequest.getLatestBaseInstant()
)
@@ -330,7 +329,7 @@ public class RollbackHelperV1 extends RollbackHelper {
.withFileExtension(HoodieLogFile.DELTA_EXTENSION);
// Apply pre-computed log version if available. Always keep the
per-task write token
- // generated above (via CommonClientUtils.generateWriteToken) so
that retried/repeated
+ // generated above (via FSUtils.makeWriteToken) so that
retried/repeated
// rollbacks do not collide on UNKNOWN_WRITE_TOKEN or inherit a
prior log's write token.
//
// When doDelete=true, we actually create a new rollback log file:
explicitly bump the
diff --git
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/util/CommonClientUtils.java
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/util/CommonClientUtils.java
index 0c8d17809ab4..90b5975252d8 100644
---
a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/util/CommonClientUtils.java
+++
b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/util/CommonClientUtils.java
@@ -21,15 +21,12 @@
package org.apache.hudi.util;
import org.apache.hudi.avro.model.HoodieClusteringPlan;
-import org.apache.hudi.common.engine.TaskContextSupplier;
-import org.apache.hudi.common.fs.FSUtils;
import org.apache.hudi.common.model.HoodieCommitMetadata;
import org.apache.hudi.common.model.HoodieFileFormat;
import org.apache.hudi.common.model.HoodieFileGroupId;
import org.apache.hudi.common.model.HoodieWriteStat;
import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.common.table.HoodieTableVersion;
-import org.apache.hudi.common.table.log.HoodieLogFormat;
import org.apache.hudi.common.table.log.block.HoodieLogBlock;
import org.apache.hudi.common.util.ClusteringUtils;
import org.apache.hudi.common.util.Option;
@@ -144,19 +141,6 @@ public class CommonClientUtils {
return
writeConfig.getWriteVersion().greaterThanOrEquals(HoodieTableVersion.TEN);
}
- public static String generateWriteToken(TaskContextSupplier
taskContextSupplier) {
- try {
- return FSUtils.makeWriteToken(
- taskContextSupplier.getPartitionIdSupplier().get(),
- taskContextSupplier.getStageIdSupplier().get(),
- taskContextSupplier.getAttemptIdSupplier().get()
- );
- } catch (Throwable t) {
- log.warn("Error generating write token, using default.", t);
- return HoodieLogFormat.DEFAULT_WRITE_TOKEN;
- }
- }
-
public static <O> HoodieWriteMetadata
stitchCompactionHoodieWriteStats(HoodieWriteMetadata<O> writeMetadata,
List<HoodieWriteStat> writeStats) {
// Fetch commit metadata from HoodieWriteMetadata and update
HoodieWriteStat
HoodieCommitMetadata commitMetadata =
writeMetadata.getCommitMetadata().get();
diff --git
a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/TestCommonClientUtils.java
b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/TestCommonClientUtils.java
index 573ccf6523be..59429de2b0d1 100644
---
a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/TestCommonClientUtils.java
+++
b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/TestCommonClientUtils.java
@@ -20,7 +20,6 @@
package org.apache.hudi.utils;
-import org.apache.hudi.common.engine.TaskContextSupplier;
import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.common.table.HoodieTableVersion;
import org.apache.hudi.config.HoodieWriteConfig;
@@ -92,16 +91,6 @@ class TestCommonClientUtils {
);
}
- @Test
- void testGenerateTokenOnError() {
- // given: a task context supplies that throws errors.
- TaskContextSupplier taskContextSupplier = mock(TaskContextSupplier.class);
- when(taskContextSupplier.getPartitionIdSupplier()).thenThrow(new
RuntimeException("generated under testing"));
-
- // when:
- assertEquals("0-0-0",
CommonClientUtils.generateWriteToken(taskContextSupplier));
- }
-
@ParameterizedTest(name = "Write version {0} should write native log format:
{1}")
@MethodSource("provideWriteVersionNativeLogExpectations")
void testShouldWriteNativeLogs(HoodieTableVersion writeVersion, boolean
expected) {
diff --git a/hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java
b/hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java
index 75b6eada4f55..448438aee017 100644
--- a/hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java
+++ b/hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java
@@ -23,11 +23,13 @@ import org.apache.hudi.avro.model.HoodieFileStatus;
import org.apache.hudi.avro.model.HoodiePath;
import org.apache.hudi.common.config.HoodieMetadataConfig;
import org.apache.hudi.common.engine.HoodieEngineContext;
+import org.apache.hudi.common.engine.TaskContextSupplier;
import org.apache.hudi.common.model.HoodieFileFormat;
import org.apache.hudi.common.model.HoodieLogFile;
import org.apache.hudi.common.model.LogExtensions;
import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
import org.apache.hudi.common.util.HoodieStorageUtils;
import org.apache.hudi.common.util.Option;
import org.apache.hudi.common.util.StringUtils;
@@ -103,6 +105,22 @@ public class FSUtils {
return String.format("%d-%d-%d", taskPartitionId, stageId, taskAttemptId);
}
+ /**
+ * Makes a write token from the engine's task context, falling back to the
default
+ * token if the context is unavailable (e.g. on the driver).
+ */
+ public static String makeWriteToken(TaskContextSupplier taskContextSupplier)
{
+ try {
+ return makeWriteToken(
+ taskContextSupplier.getPartitionIdSupplier().get(),
+ taskContextSupplier.getStageIdSupplier().get(),
+ taskContextSupplier.getAttemptIdSupplier().get());
+ } catch (Throwable t) {
+ log.warn("Error generating write token, using default.", t);
+ return HoodieLogFormat.DEFAULT_WRITE_TOKEN;
+ }
+ }
+
public static String makeBaseFileName(String instantTime, String writeToken,
String fileId, String fileExtension) {
return String.format("%s_%s_%s%s", fileId, writeToken, instantTime,
fileExtension);
}
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/util/TablePathUtils.java
b/hudi-common/src/main/java/org/apache/hudi/common/util/TablePathUtils.java
index d87ea014954c..55f910e2e899 100644
--- a/hudi-common/src/main/java/org/apache/hudi/common/util/TablePathUtils.java
+++ b/hudi-common/src/main/java/org/apache/hudi/common/util/TablePathUtils.java
@@ -118,7 +118,10 @@ public class TablePathUtils {
}
}
- private static StoragePath getNthParent(StoragePath path, int n) {
+ /**
+ * Returns the n-th parent of the given path (n=0 returns the path itself).
+ */
+ public static StoragePath getNthParent(StoragePath path, int n) {
StoragePath parent = path;
for (int i = 0; i < n; i++) {
parent = parent.getParent();
diff --git
a/hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java
b/hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java
index ccb993bd97a4..202a2e6a0b33 100644
---
a/hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java
+++
b/hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java
@@ -940,7 +940,7 @@ public class HoodieTableMetadataUtil {
public static SerializableBiFunction<String, Integer, Integer>
getSecondaryKeyToFileGroupMappingFunction(boolean needsSecondaryKeyExtraction) {
if (needsSecondaryKeyExtraction) {
return (recordKey, numFileGroups) -> {
- String secondaryKey =
SecondaryIndexKeyUtils.getUnescapedSecondaryKeyPrefixFromSecondaryIndexKey(recordKey);
+ String secondaryKey =
SecondaryIndexKeyUtils.getEscapedSecondaryKeyPrefixFromSecondaryIndexKey(recordKey);
return mapRecordKeyToFileGroupIndex(secondaryKey, numFileGroups);
};
}
diff --git
a/hudi-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexKeyUtils.java
b/hudi-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexKeyUtils.java
index 2b68fba85e3d..d3f6a63ea4fc 100644
---
a/hudi-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexKeyUtils.java
+++
b/hudi-common/src/main/java/org/apache/hudi/metadata/SecondaryIndexKeyUtils.java
@@ -55,12 +55,12 @@ public class SecondaryIndexKeyUtils {
// Give "<encoded secondaryKey>$<encoded primaryKey>"
// Extract <secondaryKey>
public static String getSecondaryKeyFromSecondaryIndexKey(String
secIdxRecKey) {
- return
unescapeSpecialChars(getUnescapedSecondaryKeyFromSecondaryIndexKey(secIdxRecKey));
+ return
unescapeSpecialChars(getEscapedSecondaryKeyFromSecondaryIndexKey(secIdxRecKey));
}
// Give "<encoded secondaryKey>$<encoded primaryKey>"
// Extract "<encoded secondaryKey>$"
- public static String
getUnescapedSecondaryKeyPrefixFromSecondaryIndexKey(String secIdxRecKey) {
+ public static String
getEscapedSecondaryKeyPrefixFromSecondaryIndexKey(String secIdxRecKey) {
int delimiterIndex = getSecondaryIndexKeySeparatorPosition(secIdxRecKey);
return secIdxRecKey.substring(0, delimiterIndex + 1);
}
@@ -73,7 +73,7 @@ public class SecondaryIndexKeyUtils {
// Give "<encoded secondaryKey>$<encoded primaryKey>"
// Extract "<encoded secondaryKey>"
- public static String getUnescapedSecondaryKeyFromSecondaryIndexKey(String
secIdxRecKey) {
+ public static String getEscapedSecondaryKeyFromSecondaryIndexKey(String
secIdxRecKey) {
int delimiterIndex = getSecondaryIndexKeySeparatorPosition(secIdxRecKey);
return secIdxRecKey.substring(0, delimiterIndex);
}
diff --git
a/hudi-common/src/test/java/org/apache/hudi/metadata/TestSecondaryIndexKeyUtils.java
b/hudi-common/src/test/java/org/apache/hudi/metadata/TestSecondaryIndexKeyUtils.java
index 3e0d2b5ffe58..6fea2ea11ce2 100644
---
a/hudi-common/src/test/java/org/apache/hudi/metadata/TestSecondaryIndexKeyUtils.java
+++
b/hudi-common/src/test/java/org/apache/hudi/metadata/TestSecondaryIndexKeyUtils.java
@@ -28,7 +28,7 @@ import org.junit.jupiter.params.provider.ValueSource;
import java.util.stream.Stream;
-import static
org.apache.hudi.metadata.SecondaryIndexKeyUtils.getUnescapedSecondaryKeyPrefixFromSecondaryIndexKey;
+import static
org.apache.hudi.metadata.SecondaryIndexKeyUtils.getEscapedSecondaryKeyPrefixFromSecondaryIndexKey;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -132,15 +132,15 @@ public class TestSecondaryIndexKeyUtils {
assertEquals("ab", SecondaryIndexKeyUtils.unescapeSpecialChars("\\a\\b"));
}
- // Test getUnescapedSecondaryKeyFromSecondaryIndexKey exhaustively
+ // Test getEscapedSecondaryKeyFromSecondaryIndexKey exhaustively
@ParameterizedTest(name = "Get unescaped secondary key: input='{0}',
expected='{1}'")
- @MethodSource("getUnescapedSecondaryKeyTestCases")
+ @MethodSource("getEscapedSecondaryKeyTestCases")
public void testGetUnescapedSecondaryKeyFromSecondaryIndexKey(String
encodedKey, String expectedSecondaryKey) {
- String actualSecondaryKey =
SecondaryIndexKeyUtils.getUnescapedSecondaryKeyFromSecondaryIndexKey(encodedKey);
+ String actualSecondaryKey =
SecondaryIndexKeyUtils.getEscapedSecondaryKeyFromSecondaryIndexKey(encodedKey);
assertEquals(expectedSecondaryKey, actualSecondaryKey);
}
- private static Stream<Arguments> getUnescapedSecondaryKeyTestCases() {
+ private static Stream<Arguments> getEscapedSecondaryKeyTestCases() {
return Stream.of(
// Simple cases
Arguments.of("key$value", "key"),
@@ -206,7 +206,7 @@ public class TestSecondaryIndexKeyUtils {
// Construct the key used by the writer path
String constructedKey =
SecondaryIndexKeyUtils.constructSecondaryIndexKey(secondaryKey, recordKey);
// The key used by the reader path and the key used by the writer path
have the following invariant.
- assertEquals(new SecondaryIndexPrefixRawKey(secondaryKey).encode(),
getUnescapedSecondaryKeyPrefixFromSecondaryIndexKey(constructedKey));
+ assertEquals(new SecondaryIndexPrefixRawKey(secondaryKey).encode(),
getEscapedSecondaryKeyPrefixFromSecondaryIndexKey(constructedKey));
// Extract both parts
String extractedSecondary =
SecondaryIndexKeyUtils.getSecondaryKeyFromSecondaryIndexKey(constructedKey);
diff --git
a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/fs/TestFSUtils.java
b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/fs/TestFSUtils.java
index b876377b9508..45daacb9eeb2 100644
---
a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/fs/TestFSUtils.java
+++
b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/fs/TestFSUtils.java
@@ -19,6 +19,7 @@
package org.apache.hudi.common.fs;
import org.apache.hudi.common.engine.HoodieLocalEngineContext;
+import org.apache.hudi.common.engine.TaskContextSupplier;
import org.apache.hudi.common.model.HoodieFileFormat;
import org.apache.hudi.common.model.HoodieLogFile;
import org.apache.hudi.common.table.HoodieTableMetaClient;
@@ -72,6 +73,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
/**
* Tests file system utils.
@@ -795,4 +798,15 @@ public class TestFSUtils extends HoodieCommonTestHarness {
private StoragePath getHoodieTempDir() {
return new StoragePath(baseUri.toString(), ".hoodie/.temp");
}
+
+ /**
+ * makeWriteToken(TaskContextSupplier) falls back to the default token when
the
+ * task context is unavailable.
+ */
+ @Test
+ public void testMakeWriteTokenOnError() {
+ TaskContextSupplier taskContextSupplier = mock(TaskContextSupplier.class);
+ when(taskContextSupplier.getPartitionIdSupplier()).thenThrow(new
RuntimeException("generated under testing"));
+ assertEquals("0-0-0", FSUtils.makeWriteToken(taskContextSupplier));
+ }
}
diff --git
a/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java
b/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java
index 501c43a10d71..0322135ce07a 100644
---
a/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java
+++
b/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java
@@ -26,11 +26,11 @@ import org.apache.hudi.common.table.timeline.HoodieTimeline;
import org.apache.hudi.common.table.view.FileSystemViewManager;
import org.apache.hudi.common.table.view.HoodieTableFileSystemView;
import org.apache.hudi.common.util.HoodieStorageUtils;
+import org.apache.hudi.common.util.TablePathUtils;
import org.apache.hudi.common.util.VisibleForTesting;
import org.apache.hudi.exception.HoodieException;
import org.apache.hudi.exception.TableNotFoundException;
import org.apache.hudi.hadoop.fs.HadoopFSUtils;
-import org.apache.hudi.hadoop.utils.HoodieHiveUtils;
import org.apache.hudi.hadoop.utils.HoodieInputFormatUtils;
import org.apache.hudi.storage.HoodieStorage;
import org.apache.hudi.storage.StorageConfiguration;
@@ -55,6 +55,7 @@ import java.util.stream.Collectors;
import static org.apache.hudi.common.config.HoodieCommonConfig.TIMESTAMP_AS_OF;
import static
org.apache.hudi.common.table.timeline.TimelineUtils.validateTimestampAsOf;
import static org.apache.hudi.common.util.StringUtils.nonEmpty;
+import static org.apache.hudi.hadoop.fs.HadoopFSUtils.convertToHadoopPath;
import static org.apache.hudi.hadoop.fs.HadoopFSUtils.convertToStoragePath;
/**
@@ -184,7 +185,7 @@ public class HoodieROTablePathFilter implements
Configurable, PathFilter, Serial
if (HoodiePartitionMetadata.hasPartitionMetadata(storage, storagePath)) {
HoodiePartitionMetadata metadata = new
HoodiePartitionMetadata(storage, storagePath);
metadata.readFromFS();
- baseDir = HoodieHiveUtils.getNthParent(folder,
metadata.getPartitionDepth());
+ baseDir = convertToHadoopPath(TablePathUtils.getNthParent(storagePath,
metadata.getPartitionDepth()));
} else {
baseDir = safeGetParentsParent(folder);
}
diff --git
a/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieHiveUtils.java
b/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieHiveUtils.java
index a781f3c53d09..385df53f28b8 100644
---
a/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieHiveUtils.java
+++
b/hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieHiveUtils.java
@@ -24,7 +24,6 @@ import org.apache.hudi.hadoop.utils.shims.HiveShim;
import org.apache.hudi.hadoop.utils.shims.HiveShims;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.Writable;
import org.apache.hadoop.mapred.JobConf;
import org.apache.hadoop.mapreduce.JobContext;
@@ -114,21 +113,6 @@ public class HoodieHiveUtils {
return job.getConfiguration().get(startCommitTimestampName);
}
- /**
- * Gets the n'th parent for the Path. Assumes the path has at-least n
components
- *
- * @param path
- * @param n
- * @return
- */
- public static Path getNthParent(Path path, int n) {
- Path parent = path;
- for (int i = 0; i < n; i++) {
- parent = parent.getParent();
- }
- return parent;
- }
-
/**
* Returns a list of tableNames for which hoodie.<tableName>.consume.mode is
set to incremental else returns empty List
*