From ebb280d5cf9ff356c2c337bfae385d016f76f194 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 11 Sep 2024 00:51:19 +0300 Subject: [PATCH] Don't inject Node when transactions are disabled In a recent refactoring, we split the PartitionManager into two types. While doing so, a bug was introduced where Transactions are partially re-enabled if they've been enabled and then disabled again in the past. This happens as we're setting up the transacation stack if transactions have been enabled in the past to recover any partially applied transactions before proceeding non-transactional access. Typically we'd just set up the Node and the recovery middleware. After the refactoring, we're injecting the Node also to other components which may alter their behavior as transactions have been enabled. Transactions have been previously enabled on production canary and this issue would affect that environment. --- internal/cli/gitaly/serve.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cli/gitaly/serve.go b/internal/cli/gitaly/serve.go index 25677b953e..3efb89eabd 100644 --- a/internal/cli/gitaly/serve.go +++ b/internal/cli/gitaly/serve.go @@ -478,7 +478,7 @@ func run(appCtx *cli.Context, cfg config.Cfg, logger log.Logger) error { } defer dbMgr.Close() - node, err = nodeimpl.NewManager( + nodeMgr, err := nodeimpl.NewManager( cfg.Storages, storagemgr.NewFactory( logger, @@ -495,9 +495,9 @@ func run(appCtx *cli.Context, cfg config.Cfg, logger log.Logger) error { if err != nil { return fmt.Errorf("new node: %w", err) } - defer node.Close() + defer nodeMgr.Close() - recoveryMiddleware := storagemgr.NewTransactionRecoveryMiddleware(protoregistry.GitalyProtoPreregistered, node) + recoveryMiddleware := storagemgr.NewTransactionRecoveryMiddleware(protoregistry.GitalyProtoPreregistered, nodeMgr) txMiddleware = server.TransactionMiddleware{ UnaryInterceptor: recoveryMiddleware.UnaryServerInterceptor(), StreamInterceptor: recoveryMiddleware.StreamServerInterceptor(), -- GitLab