Handle repository creations, deletions and renames atomically

Renaming, creating and deleting repositories is racy in Praefect. They can also partially fail in awkward ways due to Praefect first applying the operations on the disks of the Gitalys and later updating its metadata store. In order to make these operations atomic, there's been ongoing work in the background to make it possible to perform these in the database in a manner that races are not possible and partial failures do not end up conflicting with future operations.

Renames can be fixed by doing the rename atomically in the database without moving any repositories on the disks.

Deletes can be modeled as simply deleting the repository's database record.

Creates are atomic by creating the repository's database record as the last step in the process.

The last piece of the puzzle is to ensure repositories always land in different directories on the disk. This ensures that a partial failure doesn't block a future operation from succeeding. This commit implements that piece by deriving a unique path for each repository to store their replicas. Existing repositories stay where they are but newly created repositories will land in unique directories.

Create might succeed on a disk but fail to be persisted in the database. Prior to this change, attempting to recreate a repository might fail due to the stale state on the disk. With this in place, the next attempt at creating the repository would still succeed as the new attempt would land the repository in a different directory on the disk.

Deletes have the same problem prior to this commit. The repository's metadata may be successfully deleted but if we fail to delete the repository from the disk, future creations and renames may fail if they conflict with the stale state. As creates now always land in a different directory, the stale state no longer causes problems.

Renames will work purely in the database, so any stale state on the disk will not affect them.

Closes #3485 (closed)
Closes #4003 (closed)

cmd/praefect/subcmd_remove_repository.go

@@ -7,7 +7,6 @@ import (
 	"flag"
 	"fmt"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -91,16 +90,6 @@ func (cmd *removeRepository) exec(ctx context.Context, logger logrus.FieldLogger
 		return fmt.Errorf("get praefect address from config: %w", err)
 	}
 
-	logger.Debugf("remove repository info from praefect database %q", addr)
-	removed, err := cmd.removeRepositoryFromDatabase(ctx, db)
-	if err != nil {
-		return fmt.Errorf("remove repository info from praefect database: %w", err)
-	}
-	if !removed {
-		logger.Warn("praefect database has no info about the repository")
-	}
-	logger.Debug("removal of the repository info from praefect database completed")
-
 	logger.Debug("remove replication events")
 	ticker := helper.NewTimerTicker(time.Second)
 	defer ticker.Stop()
@@ -109,10 +98,15 @@ func (cmd *removeRepository) exec(ctx context.Context, logger logrus.FieldLogger
 	}
 	logger.Debug("replication events removal completed")
 
-	// We should try to remove repository from each of gitaly nodes.
-	logger.Debug("remove repository directly by each gitaly node")
-	cmd.removeRepositoryForEachGitaly(ctx, cfg, logger)
-	logger.Debug("direct repository removal by each gitaly node completed")
+	_, err = cmd.removeRepository(ctx, &gitalypb.Repository{
+		StorageName:  cmd.virtualStorage,
+		RelativePath: cmd.relativePath,
+	}, addr, cfg.Auth.Token)
+	if err != nil {
+		return fmt.Errorf("repository removal failed: %w", err)
+	}
+
+	logger.Debug("repository removal completed")
 
 	return nil
 }
@@ -203,29 +197,3 @@ func (cmd *removeRepository) removeReplicationEvents(ctx context.Context, logger
 	}
 	return nil
 }
-
-func (cmd *removeRepository) removeRepositoryForEachGitaly(ctx context.Context, cfg config.Config, logger logrus.FieldLogger) {
-	for _, vs := range cfg.VirtualStorages {
-		if vs.Name == cmd.virtualStorage {
-			var wg sync.WaitGroup
-			for i := 0; i < len(vs.Nodes); i++ {
-				wg.Add(1)
-				go func(node *config.Node) {
-					defer wg.Done()
-					logger.Debugf("remove repository with gitaly %q at %q", node.Storage, node.Address)
-					repo := &gitalypb.Repository{
-						StorageName:  node.Storage,
-						RelativePath: cmd.relativePath,
-					}
-					_, err := cmd.removeRepository(ctx, repo, node.Address, node.Token)
-					if err != nil {
-						logger.WithError(err).Warnf("repository removal failed for gitaly %q", node.Storage)
-					}
-					logger.Debugf("repository removal call to gitaly %q completed", node.Storage)
-				}(vs.Nodes[i])
-			}
-			wg.Wait()
-			break
-		}
-	}
-}