yihua commented on code in PR #11805:
URL: https://github.com/apache/hudi/pull/11805#discussion_r1744641692
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/ListingBasedRollbackStrategy.java:
##########
@@ -272,17 +287,30 @@ private FileStatus[]
fetchFilesFromCommitMetadata(HoodieInstant instantToRollbac
* @param partitionPath
* @param basePath
* @param baseFileExtension
- * @param fs
+ * @param storage
* @return
* @throws IOException
*/
- private FileStatus[] fetchFilesFromListFiles(HoodieInstant
instantToRollback, String partitionPath, String basePath,
- String baseFileExtension,
FileSystem fs)
- throws IOException {
- SerializablePathFilter pathFilter =
getSerializablePathFilter(baseFileExtension, instantToRollback.getTimestamp());
- Path[] filePaths = listFilesToBeDeleted(basePath, partitionPath);
-
- return fs.listStatus(filePaths, pathFilter);
+ private List<StoragePathInfo> fetchFilesFromListFiles(HoodieInstant
instantToRollback,
+ String partitionPath,
+ String basePath,
+ String
baseFileExtension,
+ HoodieStorage storage)
{
+ StoragePathFilter pathFilter = getPathFilter(baseFileExtension,
instantToRollback.getTimestamp());
+ StoragePath[] filePaths = listFilesToBeDeleted(basePath, partitionPath);
+
+ return Arrays.stream(filePaths)
+ .map(path -> {
+ try {
+ return storage.listDirectEntries(path, pathFilter);
+ } catch (IOException ioe) {
+ LOG.error("Failed to get StoragePathInfo for " + path.toString(),
ioe);
+ }
+ return null;
+ })
Review Comment:
Might be good to add a new API `HoodieStorage#listDirectEntries` that takes
a list of paths and do this logic, since this is used multiple times.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieCreateHandle.java:
##########
@@ -245,7 +243,7 @@ protected void setupWriteStatus() throws IOException {
stat.setPath(new StoragePath(config.getBasePath()), path);
stat.setTotalWriteErrors(writeStatus.getTotalErrorRecords());
- long fileSize = HadoopFSUtils.getFileSize(fs, new Path(path.toUri()));
+ long fileSize = storage.getPathInfo(new
StoragePath(path.toUri())).getLength();
Review Comment:
`path` is already instance of `StoragePath`.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandle.java:
##########
@@ -439,7 +437,7 @@ public List<WriteStatus> close() {
fileWriter.close();
fileWriter = null;
- long fileSizeInBytes = HadoopFSUtils.getFileSize(fs, new
Path(newFilePath.toUri()));
+ long fileSizeInBytes = storage.getPathInfo(new
StoragePath(newFilePath.toUri())).getLength();
Review Comment:
Same here. `newFilePath` is already instance of `StoragePath`.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -49,17 +48,17 @@ public class UpgradeDowngrade {
private HoodieTableMetaClient metaClient;
protected HoodieWriteConfig config;
protected HoodieEngineContext context;
- private Path updatedPropsFilePath;
- private Path propsFilePath;
+ private StoragePath updatedPropsFilePath;
+ private StoragePath propsFilePath;
public UpgradeDowngrade(
HoodieTableMetaClient metaClient, HoodieWriteConfig config,
HoodieEngineContext context,
SupportsUpgradeDowngrade upgradeDowngradeHelper) {
this.metaClient = metaClient;
this.config = config;
this.context = context;
- this.updatedPropsFilePath = new Path(metaClient.getMetaPath().toString(),
HOODIE_UPDATED_PROPERTY_FILE);
- this.propsFilePath = new Path(metaClient.getMetaPath().toString(),
HoodieTableConfig.HOODIE_PROPERTIES_FILE);
+ this.updatedPropsFilePath = new
StoragePath(metaClient.getMetaPath().toString(), HOODIE_UPDATED_PROPERTY_FILE);
+ this.propsFilePath = new StoragePath(metaClient.getMetaPath().toString(),
HoodieTableConfig.HOODIE_PROPERTIES_FILE);
Review Comment:
`.toString()` is redundant
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieCDCLogger.java:
##########
@@ -196,7 +195,7 @@ private void flushIfNeeded(Boolean force) {
HoodieLogBlock block = new HoodieCDCDataBlock(records,
cdcDataBlockHeader, keyField);
AppendResult result =
cdcWriter.appendBlocks(Collections.singletonList(block));
- Path cdcAbsPath = new Path(result.logFile().getPath().toUri());
+ StoragePath cdcAbsPath = new
StoragePath(result.logFile().getPath().toUri());
Review Comment:
Can `result.logFile().getPath()` directly be used?
--
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]
