vinishjail97 opened a new pull request, #19735:
URL: https://github.com/apache/hudi/pull/19735
### Describe the issue this Pull Request addresses
`RollbackHelperV1.addMissingLogFilesAndGetRollbackStats` builds its
executor-side `HoodieStorage` with the **no-path** overload, then uses it to
list a partition it has just computed the full path for:
```java
StoragePath fullPartitionPath = StringUtils.isNullOrEmpty(partition) ? new
StoragePath(basePathStr) : new StoragePath(basePathStr, partition);
HoodieStorage storage = HoodieStorageUtils.getStorage(storageConfiguration);
// <-- no path
List<Option<StoragePathInfo>> storagePathInfoOpts =
getPathInfoUnderPartition(storage,
fullPartitionPath, new HashSet<>(missingLogFiles), true);
```
That overload is hard-coded to the local filesystem:
```java
// HoodieStorageUtils
public static final String DEFAULT_URI = "file:///";
public static HoodieStorage getStorage(StorageConfiguration<?> conf) {
return getStorage(DEFAULT_URI, conf);
}
```
`FileSystem.get(URI, conf)` selects the implementation from the URI scheme,
so `file:///` yields a `LocalFileSystem` no matter what the configuration
carries. The executor then lists a real `s3a://` or `gs://` partition through a
`file:///`-bound handle, and `FileSystem.checkPath` rejects it:
```
org.apache.hudi.exception.HoodieRollbackException: Failed to rollback
s3a://<bucket>/<db>/<table> commits 20260825141942151
Caused by: org.apache.spark.SparkException: Job aborted due to stage
failure: Task 46 in stage 565.0 failed 4 times ...
java.lang.IllegalArgumentException: Wrong FS:
s3a://<bucket>/<db>/<table>/<partition>, expected: file:///
at org.apache.hadoop.fs.FileSystem.checkPath(FileSystem.java:807)
at
org.apache.hadoop.fs.RawLocalFileSystem.listStatus(RawLocalFileSystem.java:593)
at
org.apache.hudi.storage.hadoop.HoodieHadoopStorage.listDirectEntries(HoodieHadoopStorage.java:208)
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.getPathInfoUnderPartition(RollbackHelperV1.java:184)
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.lambda$addMissingLogFilesAndGetRollbackStats$...
Driver stacktrace:
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.addLogFilesFromPreviousFailedRollbacksToStat(RollbackHelperV1.java:459)
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.performRollback(RollbackHelperV1.java:262)
```
#### Why this branch is rarely reached
The bad line runs only when the recovered log path set is non-empty, that is
when an earlier rollback attempt was interrupted and left APPEND markers under
the rollback instant:
```java
// performRollback
logPaths = markerHandler.getAppendedLogPaths(context,
config.getFinalizeWriteParallelism());
```
```java
// addLogFilesFromPreviousFailedRollbacksToStat
if (logPaths.isEmpty()) {
// if rollback is not failed and re-attempted, we should not find any
additional log files here.
return originalRollbackStats;
}
```
On a first, uninterrupted rollback the method returns early and the defect
never executes. Existing tests also run against local `file://` base paths,
where `DEFAULT_URI` happens to be the correct filesystem, so the defect is
invisible to them by construction.
#### Why it does not self-heal
`BaseHoodieTableServiceClient.rollback` resumes a pending rollback rather
than scheduling a new one:
```java
if (pendingRollbackInfo.isPresent()) {
rollbackPlanOption =
Option.of(pendingRollbackInfo.get().getRollbackPlan());
rollbackInstantTime =
pendingRollbackInfo.get().getRollbackInstant().requestedTime();
} else {
rollbackInstantTime = suppliedRollbackInstantTime.orElseGet(() ->
createNewInstantTime(false));
rollbackPlanOption = table.scheduleRollback(...);
}
```
Same instant, same stored plan, same non-empty marker set on every retry, so
the rollback fails identically forever. Everything queued behind it is blocked
with it: clean fails outright, and compaction stays blocked behind the
un-rolled-back instant.
### Summary and Changelog
Use the path-aware `getStorage(StoragePath, StorageConfiguration)` overload
so the filesystem is resolved from the partition's own scheme.
`fullPartitionPath` is already computed on the preceding line.
```diff
- HoodieStorage storage =
HoodieStorageUtils.getStorage(storageConfiguration);
+ // Resolve storage from the partition path, not the no-path
overload: the latter binds to
+ // HoodieStorageUtils.DEFAULT_URI ("file:///"), so listing an
s3a/gs partition on an executor
+ // throws "Wrong FS ... expected: file:///" and the rollback
can never complete.
+ HoodieStorage storage =
HoodieStorageUtils.getStorage(fullPartitionPath, storageConfiguration);
```
This makes the call site consistent with
`DirectWriteMarkersV1.getAppendedLogPaths`, which already resolves storage from
the path it is about to read. It is the only remaining
`HoodieStorageUtils.getStorage(conf)` call in `RollbackHelperV1`; every other
site in the class uses `metaClient.getStorage()` or the path-aware overload.
`RollbackHelper` (table version eight and above) never calls the no-path
overload at all, which is why only the V1 path is affected.
Changes:
1. `RollbackHelperV1.addMissingLogFilesAndGetRollbackStats` resolves storage
from `fullPartitionPath`.
2. New test `TestRollbackHelperV1`, described below.
3. `HoodieRollbackTestBase` gains `createBasePath()` and
`createStorageConf()` hooks so a subclass can place the table on a scheme other
than `file`. Both defaults reproduce the previous behaviour exactly, so
`TestRollbackHelper` is unaffected.
No code was copied from elsewhere.
### Impact
`RollbackHelperFactory` routes only tables **below table version eight** to
`RollbackHelperV1`, so this affects table version six and below on non-local
storage. Such a table currently cannot complete a rollback that was interrupted
after it began appending rollback command blocks, and cannot clean or compact
for as long as that rollback stays pending. After this change it recovers on
the next attempt.
No public API, configuration, or on-disk format change.
### Risk Level
low
The change is one argument on one line, and it moves a call site onto the
same overload the rest of the class already uses. The failure it removes is
unconditional on affected tables, so there is no behaviour to preserve on the
old path.
Verification:
- The new regression test was confirmed against the unfixed code. Reverting
the one-line change fails
`testPerformRollbackAddsBackLogFilesLeftByAnInterruptedAttempt` with the
reported stack, frame for frame:
```
Caused by: java.lang.IllegalArgumentException: Wrong FS:
s3a://test-bucket/.../partition1, expected: file:///
at org.apache.hadoop.fs.FileSystem.checkPath(FileSystem.java:779)
at
org.apache.hadoop.fs.RawLocalFileSystem.listStatus(RawLocalFileSystem.java:450)
at
org.apache.hudi.storage.hadoop.HoodieHadoopStorage.listDirectEntries(HoodieHadoopStorage.java:208)
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.getPathInfoUnderPartition(RollbackHelperV1.java:184)
at
org.apache.hudi.table.action.rollback.RollbackHelperV1.lambda$addMissingLogFilesAndGetRollbackStats$...
```
- `mvn test checkstyle:check -pl hudi-client/hudi-client-common -am
-Dspark3.5
-Dtest='TestRollbackHelperV1,TestRollbackHelper,TestRollbackUtils,TestMarkerBasedRollbackStrategy'`
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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]
