SteNicholas commented on code in PR #3879:
URL: https://github.com/apache/celeborn/pull/3879#discussion_r4324814006
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/TierWriter.scala:
##########
@@ -691,39 +699,36 @@ class DfsTierWriter(
}
}
+ // A copy whose data stream or index failed to close is unreadable, so it
must not count as
+ // committed. S3/OSS upload completion failures are unchecked and already
fail the commit.
+ override protected def failCommitOnCloseStreamsError: Boolean = true
Review Comment:
@ewoodbury, propagate HDFS append-close failures before releasing replicas.
With `celeborn.worker.reuse.hdfs.outputStream.enabled=false` (the default),
`hdfsStream` is already null here. Data streams are closed inside
`DfsFlushTask.flush`, which still catches and logs `IOException` without
notifying the writer, so this new flag does not cover the default HDFS write
path.
I reproduced this on `f4048b1` by injecting an append-stream close failure
with the data left unpersisted: `close()` returned `1024`, `committed=true`,
the data file contained `0` bytes, and `.success` existed. If this happens to
the primary while its replica commits successfully, the new driver-side
selection can unpublish and delete the healthy replica, leaving the unreadable
primary.
Please propagate per-flush stream-close failures through the flush notifier,
preserving any earlier write failure, and add a regression test with stream
reuse disabled.
--
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]
