jasperjiaguo commented on code in PR #14883:
URL: https://github.com/apache/pinot/pull/14883#discussion_r1927455137
##########
pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/LocalPinotFS.java:
##########
@@ -193,19 +200,76 @@ private static File toFile(URI uri) {
private static void copy(File srcFile, File dstFile, boolean recursive)
throws IOException {
- if (dstFile.exists()) {
- FileUtils.deleteQuietly(dstFile);
+ boolean oldFileExists = dstFile.exists(); // Automatically set
oldFileExists if the old file exists
+
+ // Step 1: Calculate CRC of srcFile/directory
+ long srcCrc = calculateCrc(srcFile);
+ File backupFile = null;
+
+ if (oldFileExists) {
+ // Step 2: Rename destination file if it exists
+ backupFile = new File(dstFile.getAbsolutePath() + BACKUP);
+ if (!dstFile.renameTo(backupFile)) {
+ throw new IOException("Failed to rename destination file to backup.");
+ }
}
- if (srcFile.isDirectory()) {
- if (recursive) {
- FileUtils.copyDirectory(srcFile, dstFile);
+
+ // If any error occurs during the copy process, we want to restore the
destination file from the backup
+ try {
+ // Step 3: Copy the file or directory
+ if (srcFile.isDirectory()) {
+ if (recursive) {
+ FileUtils.copyDirectory(srcFile, dstFile);
+ } else {
+ throw new IOException(srcFile.getAbsolutePath() + " is a directory
and recursive copy is not enabled.");
+ }
+ } else {
+ FileUtils.copyFile(srcFile, dstFile);
+ }
+
+ // Step 4: Verify CRC of copied file
+ long dstCrc = calculateCrc(dstFile);
+ if (srcCrc != dstCrc) {
+
+ throw new IOException("CRC mismatch: source and destination files are
not identical.");
+ }
+ } catch (IOException e) {
+ // Restore destination file from backup if copy fails
+ if (oldFileExists) {
+ if (!dstFile.delete() || !backupFile.renameTo(dstFile)) {
+ throw new IOException("Failed to restore destination file from
backup after failed copy.");
+ }
} else {
- // Throws Exception on failure
- throw new IOException(srcFile.getAbsolutePath() + " is a directory and
recursive copy is not enabled.");
+ dstFile.delete();
+ }
+ throw e;
+ }
+
+ if (oldFileExists) {
Review Comment:
Yes the oncall will need to potentially rename the backed up segment.
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
