refactor: debounce writing to disk

[cpvalente]

The idea of this PR is to debounce writing to the disk
this means we accept a window were a crash could lose data since the last write (as of now, 3 seconds)

we expect that this would give improvements to batch operations like import
we could consider widening the window to about 20 seconds which would likely give us a good performance when people are filling in the rundowns at the added risk of data loss

[coderabbitai]

Walkthrough

The changes implement a debounced persistence mechanism for database writes, coalescing rapid persist() calls into a single operation after a 3-second quiet period. The updateBackgroundRundown function is converted to async with awaited calls in the service layer. A new flushPendingWrites() method ensures pending writes complete during application shutdown.

Changes

Cohort / File(s)	Summary
Debounced Persistence Infrastructure apps/server/src/classes/data-provider/DataProvider.ts	Introduces module-level debounced write handling via pendingWrite, activeWrite, and writeDelayMs state. The persist() method now reschedules writes after a 3-second quiet period and queues writes sequentially. New flushPendingWrites() method forces immediate completion of pending or in-progress writes.
Shutdown Integration apps/server/src/app.ts	Imports flushPendingWrites from DataProvider and calls it during application shutdown to ensure pending database writes are flushed before exit, with error suppression to avoid shutdown failures.
Rundown API Updates apps/server/src/api-data/rundown/rundown.dao.ts, apps/server/src/api-data/rundown/rundown.service.ts	Converts updateBackgroundRundown() to an async function with synchronous direct persistence via data provider calls. Service layer updated to await the async calls in editCustomField() and deleteCustomField() instead of fire-and-forget invocation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: update data in background rundowns #1959: Fixes the id parameter passed to updateBackgroundRundown() calls, while this PR makes the function async and updates all callers to await it.

Suggested labels

priority

Suggested reviewers

• alex-Arc

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	⚠️ Unable to check for merge conflicts: Failed to fetch base branch: From https://github.com/cpvalente/ontime ! [rejected] master -> master (non-fast-forward) + b6a01a5...31fd8e5 master -> origin/master (forced update)

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'refactor: debounce writing to disk' directly and concisely describes the main change: introducing debounced disk writes to improve performance.
Description check	✅ Passed	The description clearly explains the purpose of debouncing writes, the current 3-second window, the expected performance benefits for batch operations, and future optimization possibilities.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch debounce-disk

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch debounce-disk
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

apps/server/src/classes/data-provider/DataProvider.ts (2)

192-218: persist() silently swallows errors and never signals write failures to callers.

Since await persist() resolves immediately (the actual write is deferred inside setTimeout), callers like setProjectData have no way to learn that a write failed. The console.error on line 213 is the only signal, with no structured logging via the project's logger. Consider using the existing logger for consistency with the rest of the codebase, and optionally emitting a metric/event so operators can alert on persistent write failures.

Also, a minor clarity point: the JSDoc says "trailing-edge debounce" but persist() is async and every caller awaits it — this suggests the caller is waiting for the write, when in reality it resolves instantly. A brief inline note (or making persist non-async returning void) would reduce future confusion.

Suggested improvements

-async function persist() {
+function persist() {
   if (isTest) return;
 
   // Cancel any pending write and reschedule
   if (pendingWrite) {
     clearTimeout(pendingWrite);
   }
 
-  // Schedule new write after quiet period
+  // Schedule new write after quiet period (fire-and-forget; await is a no-op)
   pendingWrite = setTimeout(async () => {
     pendingWrite = null;
 
     // Wait for any in-progress write to finish first
     if (activeWrite) {
       await activeWrite;
     }
 
     try {
       activeWrite = db.write();
       await activeWrite;
     } catch (error) {
-      console.error('Failed to persist database:', error);
+      // Use the structured logger so failures are visible in production logs
+      logger.error(LogOrigin.Server, `Failed to persist database: ${error}`);
     } finally {
       activeWrite = null;
     }
   }, writeDelayMs);
 }

---

223-237: flushPendingWrites always calls db.write() even when there are no pending changes.

If neither pendingWrite nor activeWrite is set, the data on disk is already up-to-date, yet line 236 forces an unconditional write. This is harmless but adds unnecessary I/O during clean shutdowns or redundant flush calls.

Guard the final write

 export async function flushPendingWrites() {
   if (isTest) return;
 
+  let hadPending = false;
   if (pendingWrite) {
     clearTimeout(pendingWrite);
     pendingWrite = null;
+    hadPending = true;
   }
 
   // Wait for any in-progress write to finish
   if (activeWrite) {
     await activeWrite;
   }
 
-  await db.write();
+  if (hadPending) {
+    await db.write();
+  }
 }

apps/server/src/app.ts (1)

269-271: Consider logging the swallowed flush error for post-mortem debugging.

Silently discarding the error during shutdown makes it harder to diagnose data-loss scenarios. A console.warn or logger.warning here would still allow shutdown to proceed while leaving a breadcrumb.

Log the error

-  await flushPendingWrites().catch((_error) => {
-    /** nothing do to here */
-  });
+  await flushPendingWrites().catch((error) => {
+    consoleError(`Failed to flush pending writes during shutdown: ${error}`);
+  });

apps/server/src/api-data/rundown/rundown.dao.ts (2)

694-695: Same unawaited promise pattern in init().

Consistent with commit() — the setRundown call is fire-and-forget. Same suggestion: prefix with void to make intent explicit.

   // defer writing to the database
-  getDataProvider().setRundown(cachedRundown.id, cachedRundown);
+  void getDataProvider().setRundown(cachedRundown.id, cachedRundown);

---

133-143: Unawaited promises from setRundown / setCustomFields — consider explicit void annotation for clarity.

commit() is synchronous, yet setRundown and setCustomFields are async (both return Promise). The returned promises are silently discarded on lines 135 and 142. While the debounce design means the promise resolves almost immediately (before I/O), fire-and-forget patterns on promises can become unhandled rejections if refactored later or if structuredClone throws asynchronously.

Add a void annotation to make the fire-and-forget intentional and explicit:

Explicit void annotation

       // persist after all mutations are applied
-      getDataProvider().setRundown(cachedRundown.id, cachedRundown);
+      void getDataProvider().setRundown(cachedRundown.id, cachedRundown);
     }
 
     // if the customFields are mutable we persist the changes
     if (options.mutableCustomFields) {
       projectCustomFields = customFields;
 
       // persist after reassignment
-      getDataProvider().setCustomFields(projectCustomFields);
+      void getDataProvider().setCustomFields(projectCustomFields);
     }

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cpvalente]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.