chore(db): log lock metadata write failures and explain lock file lifetime

Christian Rocha and Charm Crush created 4 weeks ago

Address review feedback on the data directory lock. The metadata
written into the lock file is informational only, but silently
swallowing a write failure leaves a future contender with no clues to
report. Log it at debug level. Also leave a comment next to the lock
acquisition explaining why we intentionally do not delete the lock
file on release: flock is keyed by inode, and any close-then-unlink
ordering opens a race where two processes can each hold a flock on a
different inode that lives at the same path.

Co-Authored-By: Charm Crush <[e-mail]>

Change summary

internal/db/datadirlock.go | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

Detailed changes

internal/db/datadirlock.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -63,13 +64,17 @@ func acquireDataDirLock(dataDir string) (*dataDirLock, error) {
 	// Record ownership metadata so a contending process can identify
 	// us. Failures here are non-fatal: the OS-level lock is what
 	// actually guarantees mutual exclusion, and a missing/partial JSON
-	// payload only degrades diagnostics.
+	// payload only degrades the diagnostic a contender prints.
 	if err := writeOwnerInfo(path); err != nil {
-		// Best-effort; log via stderr only when running in a debug
-		// context would be invasive here, so we silently swallow.
-		_ = err
+		slog.Debug("Failed to write data-dir owner info", "path", path, "error", err)
 	}
 
+	// The lock file itself is intentionally never unlinked. flock is
+	// keyed by inode, not by path, and any close-then-unlink (or
+	// unlink-then-close) ordering opens a window where two processes
+	// can each hold a flock on a different inode that lives at the
+	// same path. Leaving the file in place lets every acquirer see
+	// the same inode and lets the kernel arbitrate correctly.
 	return &dataDirLock{release: release}, nil
 }