[POSIX] Drop idempotent writes (#620)

Author: AlCutter

storage/posix/README.md

@@ -32,7 +32,7 @@ The final step in the dance above is atomic according to the POSIX spec, so in p
 of actions we can avoid corrupt or partially written files being part of the tree.
 
 1. Leaves are submitted by the binary built using Tessera via a call the storage's `Add` func.
-1. The storage library batches these entries up, and, after a configurable period of time has elapsed
+1. The storage library batches these entries up in memory, and, after a configurable period of time has elapsed
    or the batch reaches a configurable size threshold, the batch is sequenced and appended to the tree:
    1. An advisory lock is taken on `.state/treeState.lock` file.
       This helps prevent multiple frontends from stepping on each other, but isn't necesary for safety.
@@ -48,6 +48,12 @@ will be updated:
 
 ## Filesystems
 
-This implementation has been somewhat tested on both a local `ext4` filesystem and on a distributed
-[CephFS](https://docs.ceph.com/en/reef/cephfs/) instance on GCP, in both cases with multiple
+This implementation has been somewhat tested on local `ext4` and `ZFS` filesystems, and on a distributed
+[CephFS](https://docs.ceph.com/en/reef/cephfs/) instance on GCP, in all cases with multiple
 personality binaries attempting to add new entries concurrently.
+
+Other POSIX compliant filesystems such as `XFS` _should_ work, but filesystems which do not offer strong
+POSIX compliance (e.g. `s3fs` or `NFS`) are unlikely to result in long term happiness.
+
+If in doubt, tools like https://github.com/saidsay-so/pjdfstest may help in determining whether a given
+filesystem is suitable.

storage/posix/files.go

@@ -396,7 +396,7 @@ func (lrs *logResourceStorage) storeTile(ctx context.Context, level, index, logS
 func (lrs *logResourceStorage) writeTile(_ context.Context, level, index uint64, partial uint8, t []byte) error {
 	tPath := layout.TilePath(level, index, partial)
 
-	if err := lrs.s.createIdempotent(tPath, t); err != nil {
+	if err := lrs.s.createOverwrite(tPath, t); err != nil {
 		return err
 	}
 
@@ -428,7 +428,7 @@ func (lrs *logResourceStorage) writeTile(_ context.Context, level, index uint64,
 // writeBundle takes care of writing out the serialised entry bundle file.
 func (lrs *logResourceStorage) writeBundle(_ context.Context, index uint64, partial uint8, bundle []byte) error {
 	bf := lrs.entriesPath(index, partial)
-	if err := lrs.s.createIdempotent(bf, bundle); err != nil {
+	if err := lrs.s.createOverwrite(bf, bundle); err != nil {
 		if !errors.Is(err, os.ErrExist) {
 			return err
 		}
@@ -525,7 +525,7 @@ func (s *Storage) writeTreeState(size uint64, root []byte) error {
 		return fmt.Errorf("error in Marshal: %v", err)
 	}
 
-	if err := overwrite(filepath.Join(s.path, stateDir, "treeState"), raw); err != nil {
+	if err := s.createOverwrite(filepath.Join(stateDir, "treeState"), raw); err != nil {
 		return fmt.Errorf("failed to create private tree state file: %w", err)
 	}
 	return nil
@@ -581,8 +581,8 @@ func (a *appender) publishCheckpoint(ctx context.Context, minStaleness time.Dura
 		return fmt.Errorf("newCP: %v", err)
 	}
 
-	if err := overwrite(filepath.Join(a.s.path, layout.CheckpointPath), cpRaw); err != nil {
-		return fmt.Errorf("overwrite(%s): %v", layout.CheckpointPath, err)
+	if err := a.s.createOverwrite(layout.CheckpointPath, cpRaw); err != nil {
+		return fmt.Errorf("createOverwrite(%s): %v", layout.CheckpointPath, err)
 	}
 
 	klog.V(2).Infof("Published latest checkpoint: %d, %x", size, root)
@@ -598,33 +598,17 @@ func (s *Storage) createExclusive(p string, d []byte) error {
 	return createEx(filepath.Join(s.path, p), d)
 }
 
+// createOverwrite atomically creates or overwrites a file at the given path with the provided data.
+func (s *Storage) createOverwrite(p string, d []byte) error {
+	return overwrite(filepath.Join(s.path, p), d)
+}
+
 func (s *Storage) readAll(p string) ([]byte, error) {
 	p = filepath.Join(s.path, p)
 	return os.ReadFile(p)
 
 }
 
-// createIdempotent atomically writes the provided data to a file at the provided path, relative to the root of the log.
-// An error will be returned if a file already exists in the named location AND that file's
-// contents is not identical to the provided data.
-func (s *Storage) createIdempotent(p string, d []byte) error {
-	if err := s.createExclusive(p, d); err != nil {
-		if errors.Is(err, os.ErrExist) {
-			r, err := s.readAll(p)
-			if err != nil {
-				return fmt.Errorf("file %q already exists, but unable to read it: %v", p, err)
-			}
-			if !bytes.Equal(d, r) {
-				return fmt.Errorf("file %q already exists but has different contents", p)
-			}
-			// Idempotent write.
-			return nil
-		}
-		return err
-	}
-	return nil
-}
-
 // stat returns os.Stat info for the speficied file relative to the log root.
 func (s *Storage) stat(p string) (os.FileInfo, error) {
 	p = filepath.Join(s.path, p)