nsivabalan commented on code in PR #18887:
URL: https://github.com/apache/hudi/pull/18887#discussion_r3532742968
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/marker/MarkerBasedRollbackUtils.java:
##########
@@ -70,10 +70,19 @@ public static List<String> getAllMarkerPaths(HoodieTable
table, HoodieEngineCont
WriteMarkers writeMarkers = WriteMarkersFactory.get(DIRECT, table,
instant);
try {
return new ArrayList<>(writeMarkers.allMarkerFilePaths());
- } catch (IOException | IllegalArgumentException e) {
- log.warn("{} not present and {} marker failed with error: {}. Falling
back to {} marker",
- MARKER_TYPE_FILENAME, DIRECT, e.getMessage(),
TIMELINE_SERVER_BASED);
- return getTimelineServerBasedMarkers(context, parallelism, markerDir,
storage);
+ } catch (IOException e) {
+ // Do NOT fall back to TIMELINE_SERVER_BASED on transient IO failures
(e.g., HDFS throttling).
+ // The timeline server looks in a different location and would return
0 markers, causing the
+ // rollback to skip deleting data files and leaving orphan files on
the table.
+ log.warn(String.format("%s not present and %s marker listing failed
with IO error: %s. "
+ + "Propagating exception — rollback will retry rather than
fall back to %s.",
+ MARKER_TYPE_FILENAME, DIRECT, e.getMessage(),
TIMELINE_SERVER_BASED));
+ throw e;
+ } catch (IllegalArgumentException e) {
+ // IllegalArgumentException indicates a marker path format mismatch —
fall back to timeline server.
+ log.warn(String.format("%s not present and %s marker failed with
error: %s. So, falling back to %s marker",
+ MARKER_TYPE_FILENAME, DIRECT, e.getMessage(),
TIMELINE_SERVER_BASED));
+ return getTimelineServerBasedMarkers(context, parallelism, markerDir,
fileSystem);
Review Comment:
we need to address this feedback as well
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/marker/MarkerBasedRollbackUtils.java:
##########
@@ -70,10 +70,19 @@ public static List<String> getAllMarkerPaths(HoodieTable
table, HoodieEngineCont
WriteMarkers writeMarkers = WriteMarkersFactory.get(DIRECT, table,
instant);
try {
return new ArrayList<>(writeMarkers.allMarkerFilePaths());
- } catch (IOException | IllegalArgumentException e) {
- log.warn("{} not present and {} marker failed with error: {}. Falling
back to {} marker",
- MARKER_TYPE_FILENAME, DIRECT, e.getMessage(),
TIMELINE_SERVER_BASED);
- return getTimelineServerBasedMarkers(context, parallelism, markerDir,
storage);
+ } catch (IOException e) {
+ // Do NOT fall back to TIMELINE_SERVER_BASED on transient IO failures
(e.g., HDFS throttling).
+ // The timeline server looks in a different location and would return
0 markers, causing the
+ // rollback to skip deleting data files and leaving orphan files on
the table.
+ log.warn(String.format("%s not present and %s marker listing failed
with IO error: %s. "
Review Comment:
hey @vamshipasunuru1 : can you address this comment
--
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]
