Merge branch 'smh-add-missing-fsyncs' into 'master'qmnguyen0711/implement-aimd-algorithm

Add missing fsyncs

Closes #5072

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5999

Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: Sami Hiltunen <shiltunen@gitlab.com>

5 files changed, 57 insertions, 1 deletions

diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go
index be2296c31..c924014e9 100644
--- a/internal/git/objectpool/pool.go
+++ b/internal/git/objectpool/pool.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/safe"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
@@ -127,7 +128,15 @@ func (o *ObjectPool) Remove(ctx context.Context) (err error) {
 		return nil
 	}
 
-	return os.RemoveAll(path)
+	if err := os.RemoveAll(path); err != nil {
+		return fmt.Errorf("remove all: %w", err)
+	}
+
+	if err := safe.NewSyncer().SyncParent(path); err != nil {
+		return fmt.Errorf("sync parent: %w", err)
+	}
+
+	return nil
 }
 
 // FromRepo returns an instance of ObjectPool that the repository points to
diff --git a/internal/git/quarantine/quarantine.go b/internal/git/quarantine/quarantine.go
index 115de6ace..4d2c0463e 100644
--- a/internal/git/quarantine/quarantine.go
+++ b/internal/git/quarantine/quarantine.go
@@ -11,6 +11,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/safe"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -104,6 +105,7 @@ func migrate(sourcePath, targetPath string) error {
 	}
 	sortEntries(entries)
 
+	syncer := safe.NewSyncer()
 	for _, entry := range entries {
 		if entry.Name() == "." {
 			continue
@@ -123,12 +125,24 @@ func migrate(sourcePath, targetPath string) error {
 				return fmt.Errorf("migrating directory %q: %w", nestedSourcePath, err)
 			}
 
+			if err := syncer.Sync(nestedTargetPath); err != nil {
+				return fmt.Errorf("sync directory: %w", err)
+			}
+
 			continue
 		}
 
 		if err := finalizeObjectFile(nestedSourcePath, nestedTargetPath); err != nil {
 			return fmt.Errorf("migrating object file %q: %w", nestedSourcePath, err)
 		}
+
+		if err := syncer.Sync(nestedTargetPath); err != nil {
+			return fmt.Errorf("sync object: %w", err)
+		}
+	}
+
+	if err := syncer.Sync(targetPath); err != nil {
+		return fmt.Errorf("sync object directory: %w", err)
 	}
 
 	if err := os.Remove(sourcePath); err != nil {
diff --git a/internal/gitaly/repoutil/create.go b/internal/gitaly/repoutil/create.go
index faade627f..650996a75 100644
--- a/internal/gitaly/repoutil/create.go
+++ b/internal/gitaly/repoutil/create.go
@@ -14,6 +14,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/safe"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/tempdir"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting"
@@ -219,12 +220,26 @@ func Create(
 		return structerr.NewFailedPrecondition("preparatory vote: %w", err)
 	}
 
+	syncer := safe.NewSyncer()
+	if err := syncer.SyncRecursive(newRepoDir.Path()); err != nil {
+		return fmt.Errorf("sync recursive: %w", err)
+	}
+
 	// Now that we have locked the repository and all Gitalies have agreed that they
 	// want to do the same change we can move the repository into place.
 	if err := os.Rename(newRepoDir.Path(), targetPath); err != nil {
 		return fmt.Errorf("moving repository into place: %w", err)
 	}
 
+	storagePath, err := locator.GetStorageByName(repository.GetStorageName())
+	if err != nil {
+		return fmt.Errorf("get storage by name: %w", err)
+	}
+
+	if err := syncer.SyncHierarchy(storagePath, repository.GetRelativePath()); err != nil {
+		return fmt.Errorf("sync hierarchy: %w", err)
+	}
+
 	if err := transaction.VoteOnContext(ctx, txManager, vote, voting.Committed); err != nil {
 		return structerr.NewFailedPrecondition("committing vote: %w", err)
 	}
diff --git a/internal/gitaly/repoutil/remove.go b/internal/gitaly/repoutil/remove.go
index 75a2ce998..de3ca7822 100644
--- a/internal/gitaly/repoutil/remove.go
+++ b/internal/gitaly/repoutil/remove.go
@@ -84,6 +84,10 @@ func Remove(
 		return structerr.NewInternal("staging repository for removal: %w", err)
 	}
 
+	if err := safe.NewSyncer().SyncParent(path); err != nil {
+		return fmt.Errorf("sync removal: %w", err)
+	}
+
 	if err := os.RemoveAll(destDir); err != nil {
 		return structerr.NewInternal("removing repository: %w", err)
 	}
diff --git a/internal/gitaly/service/repository/rename.go b/internal/gitaly/service/repository/rename.go
index b97509eba..51343e681 100644
--- a/internal/gitaly/service/repository/rename.go
+++ b/internal/gitaly/service/repository/rename.go
@@ -94,6 +94,20 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g
 		return fmt.Errorf("moving repository into place: %w", err)
 	}
 
+	storagePath, err := s.locator.GetStorageByName(targetRepo.GetStorageName())
+	if err != nil {
+		return fmt.Errorf("get storage by name: %w", err)
+	}
+
+	syncer := safe.NewSyncer()
+	if err := syncer.SyncHierarchy(storagePath, targetRepo.GetRelativePath()); err != nil {
+		return fmt.Errorf("sync hierarchy: %w", err)
+	}
+
+	if err := syncer.SyncParent(sourcePath); err != nil {
+		return fmt.Errorf("sync parent: %w", err)
+	}
+
 	return nil
 }