Feature: retry failed writes (#1088)

emmercm · Apr 12, 2024 · 89c3aa9 · 89c3aa9
1 parent 30a5f26
commit 89c3aa9
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 42 deletions.
diff --git a/src/constants.ts b/src/constants.ts
@@ -111,6 +111,11 @@ export default class Constants {
    */
   static readonly ROM_WRITER_DEFAULT_THREADS = this.FILE_READER_DEFAULT_THREADS / 2;
 
+  /**
+   * The number of additional retry attempts to write a file if the write or test fails.
+   */
+  static readonly ROM_WRITER_ADDITIONAL_RETRIES = 2;
+
   /**
    * Max number of files to recycle/delete at once.
    */

diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts
@@ -767,6 +767,14 @@ export default class ArgumentsParser {
           middlewareArgv.datThreads = 1;
         }
       }, true)
+      .option('write-retry', {
+        group: groupHelpDebug,
+        description: 'Number of additional retries to attempt when writing a file has failed (0 disables retries)',
+        type: 'number',
+        coerce: (val: number) => Math.max(val, 0),
+        requiresArg: true,
+        default: Constants.ROM_WRITER_ADDITIONAL_RETRIES,
+      })
       .option('disable-cache', {
         group: groupHelpDebug,
         description: 'Disable the file and archive entry checksum cache',

diff --git a/src/modules/candidateWriter.ts b/src/modules/candidateWriter.ts
@@ -203,21 +203,36 @@ export default class CandidateWriter extends Module {
       }
     }
 
-    if (!await this.writeZipFile(dat, releaseCandidate, outputZip, inputToOutputZipEntries)) {
-      // It's expected that an error was already logged
-      return;
-    }
-
-    if (this.options.shouldTest()) {
-      const writtenTest = await this.testZipContents(
+    for (let i = 0; i <= this.options.getWriteRetry(); i += 1) {
+      const written = await this.writeZipFile(
         dat,
         releaseCandidate,
-        outputZip.getFilePath(),
-        inputToOutputZipEntries.map((entry) => entry[1]),
+        outputZip,
+        inputToOutputZipEntries,
       );
-      if (writtenTest) {
-        this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${outputZip.getFilePath()}: written zip ${writtenTest}`);
-        return;
+
+      if (written && !this.options.shouldTest()) {
+        // Successfully written, unknown if valid
+        break;
+      }
+      if (written && this.options.shouldTest()) {
+        const writtenTest = await this.testZipContents(
+          dat,
+          releaseCandidate,
+          outputZip.getFilePath(),
+          inputToOutputZipEntries.map((entry) => entry[1]),
+        );
+        if (!writtenTest) {
+          // Successfully validated
+          break;
+        }
+        const message = `${dat.getNameShort()}: ${releaseCandidate.getName()}: ${outputZip.getFilePath()}: written zip ${writtenTest}`;
+        if (i < this.options.getWriteRetry()) {
+          this.progressBar.logWarn(`${message}, retrying`);
+        } else {
+          this.progressBar.logError(message);
+          return; // final error, do not continue
+        }
       }
     }
 
@@ -424,20 +439,31 @@ export default class CandidateWriter extends Module {
       }
     }
 
-    if (!await this.writeRawFile(dat, releaseCandidate, inputRomFile, outputFilePath)) {
-      // It's expected that an error was already logged
-      return;
-    }
-    if (this.options.shouldTest()) {
-      const writtenTest = await this.testWrittenRaw(
-        dat,
-        releaseCandidate,
-        outputFilePath,
-        outputRomFile,
-      );
-      if (writtenTest) {
-        this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${outputFilePath}: written file ${writtenTest}`);
-        return;
+    for (let i = 0; i <= this.options.getWriteRetry(); i += 1) {
+      const written = await this.writeRawFile(dat, releaseCandidate, inputRomFile, outputFilePath);
+
+      if (written && !this.options.shouldTest()) {
+        // Successfully written, unknown if valid
+        break;
+      }
+      if (written && this.options.shouldTest()) {
+        const writtenTest = await this.testWrittenRaw(
+          dat,
+          releaseCandidate,
+          outputFilePath,
+          outputRomFile,
+        );
+        if (!writtenTest) {
+          // Successfully validated
+          break;
+        }
+        const message = `${dat.getNameShort()}: ${releaseCandidate.getName()}: ${outputFilePath}: written file ${writtenTest}`;
+        if (i < this.options.getWriteRetry()) {
+          this.progressBar.logWarn(`${message}, retrying`);
+        } else {
+          this.progressBar.logError(message);
+          return; // final error, do not continue
+        }
       }
     }
     this.enqueueFileDeletion(inputRomFile);
@@ -593,6 +619,44 @@ export default class CandidateWriter extends Module {
       await fsPoly.rm(linkPath, { force: true });
     }
 
+    for (let i = 0; i <= this.options.getWriteRetry(); i += 1) {
+      const written = await this.writeRawLink(dat, releaseCandidate, targetPath, linkPath);
+
+      if (written && !this.options.shouldTest()) {
+        // Successfully written, unknown if valid
+        break;
+      }
+      if (written && this.options.shouldTest()) {
+        let writtenTest;
+        if (this.options.getSymlink()) {
+          writtenTest = await CandidateWriter.testWrittenSymlink(linkPath, targetPath);
+        } else {
+          writtenTest = await CandidateWriter.testWrittenHardlink(
+            linkPath,
+            inputRomFile.getFilePath(),
+          );
+        }
+        if (!writtenTest) {
+          // Successfully validated
+          break;
+        }
+        const message = `${dat.getNameShort()}: ${releaseCandidate.getName()} ${linkPath}: written link ${writtenTest}`;
+        if (i < this.options.getWriteRetry()) {
+          this.progressBar.logWarn(`${message}, retrying`);
+        } else {
+          this.progressBar.logError(message);
+          return; // final error, do not continue
+        }
+      }
+    }
+  }
+
+  private async writeRawLink(
+    dat: DAT,
+    releaseCandidate: ReleaseCandidate,
+    targetPath: string,
+    linkPath: string,
+  ): Promise<boolean> {
     try {
       await CandidateWriter.ensureOutputDirExists(linkPath);
       if (this.options.getSymlink()) {
@@ -602,24 +666,10 @@ export default class CandidateWriter extends Module {
         this.progressBar.logInfo(`${dat.getNameShort()}: ${releaseCandidate.getName()}: creating hard link '${targetPath}' -> '${linkPath}'`);
         await fsPoly.hardlink(targetPath, linkPath);
       }
+      return true;
     } catch (error) {
       this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${linkPath}: failed to link from ${targetPath}: ${error}`);
-      return;
-    }
-
-    if (this.options.shouldTest()) {
-      let writtenTest;
-      if (this.options.getSymlink()) {
-        writtenTest = await CandidateWriter.testWrittenSymlink(linkPath, targetPath);
-      } else {
-        writtenTest = await CandidateWriter.testWrittenHardlink(
-          linkPath,
-          inputRomFile.getFilePath(),
-        );
-      }
-      if (writtenTest) {
-        this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()} ${linkPath}: written link ${writtenTest}`);
-      }
+      return false;
     }
   }
 

diff --git a/src/types/options.ts b/src/types/options.ts
@@ -144,6 +144,7 @@ export interface OptionsProps {
   readonly datThreads?: number,
   readonly readerThreads?: number,
   readonly writerThreads?: number,
+  readonly writeRetry?: number,
   readonly disableCache?: boolean,
   readonly verbose?: number,
   readonly help?: boolean,
@@ -334,6 +335,8 @@ export default class Options implements OptionsProps {
 
   readonly writerThreads: number;
 
+  readonly writeRetry: number;
+
   readonly disableCache: boolean;
 
   readonly verbose: number;
@@ -441,6 +444,7 @@ export default class Options implements OptionsProps {
     this.datThreads = Math.max(options?.datThreads ?? 0, 1);
     this.readerThreads = Math.max(options?.readerThreads ?? 0, 1);
     this.writerThreads = Math.max(options?.writerThreads ?? 0, 1);
+    this.writeRetry = Math.max(options?.writeRetry ?? 0, 0);
     this.disableCache = options?.disableCache ?? false;
     this.verbose = options?.verbose ?? 0;
     this.help = options?.help ?? false;
@@ -1165,6 +1169,10 @@ export default class Options implements OptionsProps {
     return this.writerThreads;
   }
 
+  getWriteRetry(): number {
+    return this.writeRetry;
+  }
+
   getDisableCache(): boolean {
     return this.disableCache;
   }

diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts
@@ -1152,6 +1152,14 @@ describe('options', () => {
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--writer-threads', '2']).getWriterThreads()).toEqual(2);
   });
 
+  it('should parse "write-retry"', () => {
+    expect(argumentsParser.parse(dummyCommandAndRequiredArgs).getWriteRetry()).toEqual(2);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--write-retry', '-1']).getWriteRetry()).toEqual(0);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--write-retry', '0']).getWriteRetry()).toEqual(0);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--write-retry', '1']).getWriteRetry()).toEqual(1);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--write-retry', '2']).getWriteRetry()).toEqual(2);
+  });
+
   it('should parse "disable-cache"', () => {
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs]).getDisableCache())
       .toEqual(false);