git: Unconditionally lower big file threshold

In 34399547b (git: Lower big file threshold to improve memory use and
diff latency, 2023-08-08), we have introduced logic to lower the big
file threshold of Git from the default 512MB to 50MB. BLobs larger than
50MB will thus be treated specially: they aren't diffed, they aren't
deltified, and Git will use streaming interfaces instead of slurping any
such blobs into memory directly. This should thus both reduce compute
time and memory overhead in repositories that have such large blobs.

We have rolled out this flag into production a few days ago without any
observed negative fallout. On the other hand, we also didn't observe any
immediate positive consequences, either. This is expected though: the
amount of requests that handle such files is going to be miniscule, so
any effect caused by the lowered limit will be drowned out by all the
other requests.

Let's remove the feature flag guarding this new setting.

5 files changed, 15 insertions, 64 deletions

diff --git a/internal/featureflag/ff_lower_big_file_threshold.go b/internal/featureflag/ff_lower_big_file_threshold.go
deleted file mode 100644
index cbebdfd2f..000000000
--- a/internal/featureflag/ff_lower_big_file_threshold.go
+++ /dev/null
@@ -1,15 +0,0 @@
-package featureflag
-
-// LowerBigFileThreshold lowers the `core.bigFileThreshold` from 512MB to 50MB. This has two consequences:
-//
-//   - Files larger than 50MB are not going to be slurped into memory anymore, but will instead use streaming interfaces.
-//     This should improve memory consumption.
-//
-//   - Files larger than 50MB will not be diffed anymore. This should improve latency for RPCs that compute diffs when any
-//     such large files are involved. The downside is that diffs for such files cannot be viewed anymore.
-var LowerBigFileThreshold = NewFeatureFlag(
-	"lower_big_file_threshold",
-	"v16.3.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/5496",
-	false,
-)
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 56b0e33d9..ff4f4dfe1 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -14,7 +14,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/cgroups"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/command"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/alternates"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2hooks"
@@ -625,9 +624,7 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config
 		// still observed lock contention around them though, but mostly in cases where the host system was
 		// heavily loaded by a storm of incoming RPCs.
 		{Key: "core.filesRefLockTimeout", Value: "1000"},
-	}
 
-	if featureflag.LowerBigFileThreshold.IsEnabled(ctx) {
 		// Change the size of files we consider to be big from 512MB to 50MB. This setting influences a bunch of
 		// things for blobs that are larger than this size:
 		//
@@ -646,7 +643,7 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config
 		// So ultimately, this should not lead to larger packfiles as we have already been restricting the
 		// packfile window anyway while it should on the other hand lead to lower memory consumption and faster
 		// computation of diffs when large blobs are involved.
-		config = append(config, ConfigPair{Key: "core.bigFileThreshold", Value: fmt.Sprintf("%dm", BigFileThresholdMB)})
+		{Key: "core.bigFileThreshold", Value: fmt.Sprintf("%dm", BigFileThresholdMB)},
 	}
 
 	return config, nil
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index fa9a790d1..9d33805f8 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -18,7 +18,6 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2"
@@ -597,12 +596,8 @@ func TestExecCommandFactory_GitVersion(t *testing.T) {
 
 func TestExecCommandFactory_config(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.LowerBigFileThreshold).Run(t, testExecCommandFactoryConfig)
-}
-
-func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) {
-	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
 	// Create a repository and remove its gitconfig to bring us into a known state where there
@@ -620,11 +615,8 @@ func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) {
 		"core.fsyncmethod=fsync",
 		"core.packedrefstimeout=10000",
 		"core.filesreflocktimeout=1000",
+		"core.bigfilethreshold=50m",
 	}
-	expectedEnv = append(expectedEnv, testhelper.EnabledOrDisabledFlag(ctx, featureflag.LowerBigFileThreshold,
-		[]string{"core.bigfilethreshold=50m"},
-		[]string{},
-	)...)
 
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 
diff --git a/internal/gitaly/service/diff/commit_diff_test.go b/internal/gitaly/service/diff/commit_diff_test.go
index 45a631762..938137629 100644
--- a/internal/gitaly/service/diff/commit_diff_test.go
+++ b/internal/gitaly/service/diff/commit_diff_test.go
@@ -1,13 +1,11 @@
 package diff
 
 import (
-	"context"
 	"io"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/diff"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
@@ -18,12 +16,8 @@ import (
 
 func TestCommitDiff(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.LowerBigFileThreshold).Run(t, testCommitDiff)
-}
-
-func testCommitDiff(t *testing.T, ctx context.Context) {
-	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	cfg, client := setupDiffService(t)
 
 	type setupData struct {
@@ -538,32 +532,18 @@ func testCommitDiff(t *testing.T, ctx context.Context) {
 						LeftCommitId:  commit1.String(),
 						RightCommitId: commit2.String(),
 					},
-					expectedDiff: testhelper.EnabledOrDisabledFlag(ctx, featureflag.LowerBigFileThreshold,
-						[]*diff.Diff{
-							{
-								FromID:   blob1.String(),
-								ToID:     blob2.String(),
-								OldMode:  0o100644,
-								NewMode:  0o100644,
-								FromPath: []byte("huge"),
-								ToPath:   []byte("huge"),
-								Binary:   true,
-								Patch:    []byte("Binary files a/huge and b/huge differ\n"),
-							},
-						},
-						[]*diff.Diff{
-							{
-								FromID:   blob1.String(),
-								ToID:     blob2.String(),
-								OldMode:  0o100644,
-								NewMode:  0o100644,
-								FromPath: []byte("huge"),
-								ToPath:   []byte("huge"),
-								Binary:   false,
-								Patch:    []byte("@@ -2,4 +2,4 @@\n \n \n \n-a\n+b\n"),
-							},
+					expectedDiff: []*diff.Diff{
+						{
+							FromID:   blob1.String(),
+							ToID:     blob2.String(),
+							OldMode:  0o100644,
+							NewMode:  0o100644,
+							FromPath: []byte("huge"),
+							ToPath:   []byte("huge"),
+							Binary:   true,
+							Patch:    []byte("Binary files a/huge and b/huge differ\n"),
 						},
-					),
+					},
 				}
 			},
 		},
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 03869008d..2fa5c33f1 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -237,9 +237,6 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.RunCommandsInCGroup, true)
 	// Randomly enable mailmap
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.MailmapOptions, rand.Int()%2 == 0)
-	// LowerBigFileThreshold is checked on every spawn of Git commands and is thus infeasible to be checked for
-	// explicitly in every single test.
-	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.LowerBigFileThreshold, rand.Int()%2 == 0)
 
 	for _, opt := range opts {
 		ctx = opt(ctx)