You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "yihua (via GitHub)" <gi...@apache.org> on 2024/09/04 23:46:08 UTC
Re: [PR] [HUDI-8092] Replace FileSystem and related classes with Hudi's storage-layer classes [hudi]
yihua commented on code in PR #11805:
URL: https://github.com/apache/hudi/pull/11805#discussion_r1743107911
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/FileSystemBasedLockProvider.java:
##########
@@ -81,21 +80,21 @@ public FileSystemBasedLockProvider(final LockConfiguration lockConfiguration, fi
+ StoragePath.SEPARATOR + HoodieTableMetaClient.METAFOLDER_NAME;
}
this.lockTimeoutMinutes = lockConfiguration.getConfig().getInteger(FILESYSTEM_LOCK_EXPIRE_PROP_KEY);
- this.lockFile = new Path(lockDirectory + StoragePath.SEPARATOR + LOCK_FILE_NAME);
+ this.lockFile = new StoragePath(lockDirectory + StoragePath.SEPARATOR + LOCK_FILE_NAME);
this.lockInfo = new LockInfo();
this.sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
- this.fs = HadoopFSUtils.getFs(this.lockFile.toString(), configuration);
+ this.storage = HoodieStorageUtils.getStorage(this.lockFile.toString(), configuration);
List<String> customSupportedFSs = lockConfiguration.getConfig().getStringList(HoodieCommonConfig.HOODIE_FS_ATOMIC_CREATION_SUPPORT.key(), ",", new ArrayList<>());
- if (!customSupportedFSs.contains(this.fs.getScheme()) && !StorageSchemes.isAtomicCreationSupported(this.fs.getScheme())) {
- throw new HoodieLockException("Unsupported scheme :" + this.fs.getScheme() + ", since this fs can not support atomic creation");
+ if (!customSupportedFSs.contains(this.storage.getScheme()) && !StorageSchemes.isAtomicCreationSupported(this.storage.getScheme())) {
+ throw new HoodieLockException("Unsupported scheme :" + this.storage.getScheme() + ", since this fs can not support atomic creation");
}
}
@Override
public void close() {
synchronized (LOCK_FILE_NAME) {
try {
- fs.delete(this.lockFile, true);
+ storage.deleteDirectory(this.lockFile);
Review Comment:
Same here for using `deleteFile`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/FileSystemBasedLockProvider.java:
##########
@@ -107,17 +106,17 @@ public boolean tryLock(long time, TimeUnit unit) {
try {
synchronized (LOCK_FILE_NAME) {
// Check whether lock is already expired, if so try to delete lock file
- if (fs.exists(this.lockFile)) {
+ if (storage.exists(this.lockFile)) {
if (checkIfExpired()) {
- fs.delete(this.lockFile, true);
+ storage.deleteDirectory(this.lockFile);
Review Comment:
Same here for using `deleteFile`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/bucket/ConsistentBucketIndexUtils.java:
##########
@@ -110,30 +106,35 @@ public static HoodieConsistentHashingMetadata loadOrCreateMetadata(HoodieTable t
*/
public static Option<HoodieConsistentHashingMetadata> loadMetadata(HoodieTable table, String partition) {
HoodieTableMetaClient metaClient = table.getMetaClient();
- Path metadataPath = HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getHashingMetadataPath(), partition);
- Path partitionPath = HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getBasePath().toString(), partition);
+ StoragePath metadataPath = new StoragePath(
+ HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getHashingMetadataPath(), partition)
+ .toUri());
+ StoragePath partitionPath = new StoragePath(
+ HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getBasePath().toString(), partition)
+ .toUri());
Review Comment:
Use `FSUtils.constructAbsolutePath` directly
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/FileSystemBasedLockProvider.java:
##########
@@ -129,8 +128,8 @@ public boolean tryLock(long time, TimeUnit unit) {
public void unlock() {
synchronized (LOCK_FILE_NAME) {
try {
- if (fs.exists(this.lockFile)) {
- fs.delete(this.lockFile, true);
+ if (storage.exists(this.lockFile)) {
+ storage.deleteDirectory(this.lockFile);
Review Comment:
This should use `deleteFile` as `this.lockFile` represents a file not directory.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/bucket/ConsistentBucketIndexUtils.java:
##########
@@ -267,8 +268,9 @@ private static Option<HoodieConsistentHashingMetadata> loadMetadataFromGivenFile
* @param partition Partition metadata file belongs to
* @return true if hashing metadata file is latest else false
*/
- private static boolean recommitMetadataFile(HoodieTable table, FileStatus metaFile, String partition) {
- Path partitionPath = new Path(FSUtils.constructAbsolutePath(table.getMetaClient().getBasePath(), partition).toUri());
+ private static boolean recommitMetadataFile(HoodieTable table, StoragePathInfo metaFile, String partition) {
+ StoragePath partitionPath = new StoragePath(
+ FSUtils.constructAbsolutePath(table.getMetaClient().getBasePath(), partition).toUri());
Review Comment:
`FSUtils.constructAbsolutePath` returns storage path already.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/bucket/ConsistentBucketIndexUtils.java:
##########
@@ -110,30 +106,35 @@ public static HoodieConsistentHashingMetadata loadOrCreateMetadata(HoodieTable t
*/
public static Option<HoodieConsistentHashingMetadata> loadMetadata(HoodieTable table, String partition) {
HoodieTableMetaClient metaClient = table.getMetaClient();
- Path metadataPath = HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getHashingMetadataPath(), partition);
- Path partitionPath = HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getBasePath().toString(), partition);
+ StoragePath metadataPath = new StoragePath(
+ HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getHashingMetadataPath(), partition)
+ .toUri());
+ StoragePath partitionPath = new StoragePath(
+ HadoopFSUtils.constructAbsolutePathInHadoopPath(metaClient.getBasePath().toString(), partition)
+ .toUri());
try {
- Predicate<FileStatus> hashingMetaCommitFilePredicate = fileStatus -> {
- String filename = fileStatus.getPath().getName();
+ Predicate<StoragePathInfo> hashingMetaCommitFilePredicate = pathInfo -> {
+ String filename = pathInfo.getPath().getName();
return filename.contains(HoodieConsistentHashingMetadata.HASHING_METADATA_COMMIT_FILE_SUFFIX);
};
- Predicate<FileStatus> hashingMetadataFilePredicate = fileStatus -> {
- String filename = fileStatus.getPath().getName();
+ Predicate<StoragePathInfo> hashingMetadataFilePredicate = pathInfo -> {
+ String filename = pathInfo.getPath().getName();
return filename.contains(HASHING_METADATA_FILE_SUFFIX);
};
- final FileStatus[] metaFiles =
- ((FileSystem) metaClient.getStorage().getFileSystem()).listStatus(metadataPath);
- final TreeSet<String> commitMetaTss = Arrays.stream(metaFiles).filter(hashingMetaCommitFilePredicate)
+ final List<StoragePathInfo> metaFiles = metaClient.getStorage().listDirectEntries(metadataPath);
+ final TreeSet<String> commitMetaTss = metaFiles.stream().filter(hashingMetaCommitFilePredicate)
.map(commitFile -> HoodieConsistentHashingMetadata.getTimestampFromFile(commitFile.getPath().getName()))
.sorted()
.collect(Collectors.toCollection(TreeSet::new));
- final FileStatus[] hashingMetaFiles = Arrays.stream(metaFiles).filter(hashingMetadataFilePredicate)
+ final List<StoragePathInfo> hashingMetaFiles = metaFiles.stream().filter(hashingMetadataFilePredicate)
.sorted(Comparator.comparing(f -> f.getPath().getName()))
- .toArray(FileStatus[]::new);
+ .collect(Collectors.toList());
// max committed metadata file
final String maxCommitMetaFileTs = commitMetaTss.isEmpty() ? null : commitMetaTss.last();
// max updated metadata file
- FileStatus maxMetadataFile = hashingMetaFiles.length > 0 ? hashingMetaFiles[hashingMetaFiles.length - 1] : null;
+ StoragePathInfo maxMetadataFile = !hashingMetaFiles.isEmpty()
+ ? hashingMetaFiles.get(hashingMetaFiles.size() - 1)
+ : null;
Review Comment:
For readability
```suggestion
StoragePathInfo maxMetadataFile = hashingMetaFiles.isEmpty()
? null
: hashingMetaFiles.get(hashingMetaFiles.size() - 1);
```
--
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