Remove deprecated storage backends

Ever since we published ADR #2 which introduced LocalBlobAccess, ShardingBlobAccess and MirroredBlobAccess, the writing has been on the wall that there would no longer be any place for backends such as Redis and SizeDistinguishing. The main reason I kept these two backends around, was because people in the community were still interested in seeing S3 support reappear. Because I have not received any serious proposal from members within the community to do this in a way that it meets the standards for inclusion, I think we've now reached the point where we can assume no work is going to happen in this area in the short term. Fixes: #175 Issue: #176
buildbarn · Oct 8, 2023 · ba53c0a · ba53c0a
1 parent 0bb5e73
commit ba53c0a
Show file tree

Hide file tree

Showing 11 changed files with 847 additions and 1,884 deletions.
diff --git a/README.md b/README.md
@@ -17,10 +17,11 @@ does, however, facilitate remote execution by allowing execution
 requests to be forwarded to a separate remote execution service.
 
 This storage daemon can be configured to use a whole series of backends.
-Examples include networked systems like Redis, but also a local on-disk
-storage backend that writes data to a large file, using a hash table as
-an index. This storage backend is self-cleaning; no garbage collection
-is needed. The [schema of the storage configuration file](https://github.com/buildbarn/bb-storage/blob/master/pkg/proto/configuration/blobstore/blobstore.proto)
+Examples include a backend that forwards traffic over gRPC, but also a
+local on-disk storage backend that writes data to a large file, using a
+hash table as an index. This storage backend is self-cleaning; no
+garbage collection is needed. The
+[schema of the storage configuration file](https://github.com/buildbarn/bb-storage/blob/master/pkg/proto/configuration/blobstore/blobstore.proto)
 gives a good overview of which storage backends are available and how
 they can be configured.
 

diff --git a/internal/mock/BUILD.bazel b/internal/mock/BUILD.bazel
@@ -248,14 +248,6 @@ gomock(
     package = "mock",
 )
 
-gomock(
-    name = "redis",
-    out = "redis.go",
-    interfaces = ["RedisClient"],
-    library = "//pkg/blobstore",
-    package = "mock",
-)
-
 gomock(
     name = "remoteexecution",
     out = "remoteexecution.go",
@@ -316,7 +308,6 @@ go_library(
         "http.go",
         "jwt.go",
         "random.go",
-        "redis.go",
         "remoteexecution.go",
         "trace.go",
         "util.go",
@@ -342,7 +333,6 @@ go_library(
         "//pkg/util",
         "@com_github_aws_aws_sdk_go_v2_service_s3//:s3",
         "@com_github_bazelbuild_remote_apis//build/bazel/remote/execution/v2:execution",
-        "@com_github_go_redis_redis_v8//:redis",
         "@com_github_golang_mock//gomock",
         "@com_github_google_uuid//:uuid",
         "@com_google_cloud_go_longrunning//autogen/longrunningpb",

diff --git a/pkg/blobstore/BUILD.bazel b/pkg/blobstore/BUILD.bazel
@@ -21,9 +21,7 @@ go_library(
         "metrics_blob_access.go",
         "read_buffer_factory.go",
         "read_canarying_blob_access.go",
-        "redis_blob_access.go",
         "reference_expanding_blob_access.go",
-        "size_distinguishing_blob_access.go",
         "validation_caching_read_buffer_factory.go",
         "zip_reading_blob_access.go",
         "zip_writing_blob_access.go",
@@ -47,15 +45,13 @@ go_library(
         "@com_github_aws_aws_sdk_go_v2//aws",
         "@com_github_aws_aws_sdk_go_v2_service_s3//:s3",
         "@com_github_bazelbuild_remote_apis//build/bazel/remote/execution/v2:execution",
-        "@com_github_go_redis_redis_v8//:redis",
         "@com_github_klauspost_compress//zstd",
         "@com_github_prometheus_client_golang//prometheus",
         "@org_golang_google_grpc//codes",
         "@org_golang_google_grpc//status",
         "@org_golang_google_protobuf//encoding/protowire",
         "@org_golang_google_protobuf//proto",
         "@org_golang_google_protobuf//types/known/timestamppb",
-        "@org_golang_x_sync//errgroup",
     ],
 )
 
@@ -70,7 +66,6 @@ go_test(
         "existence_caching_blob_access_test.go",
         "hierarchical_instance_names_blob_access_test.go",
         "read_canarying_blob_access_test.go",
-        "redis_blob_access_test.go",
         "reference_expanding_blob_access_test.go",
         "validation_caching_read_buffer_factory_test.go",
         "zip_reading_blob_access_test.go",

diff --git a/pkg/blobstore/configuration/BUILD.bazel b/pkg/blobstore/configuration/BUILD.bazel
@@ -47,8 +47,6 @@ go_library(
         "@com_github_aws_aws_sdk_go_v2_service_s3//:s3",
         "@com_github_bazelbuild_remote_apis//build/bazel/remote/execution/v2:execution",
         "@com_github_fxtlabs_primes//:primes",
-        "@com_github_go_redis_redis_extra_redisotel//:redisotel",
-        "@com_github_go_redis_redis_v8//:redis",
         "@com_github_google_uuid//:uuid",
         "@com_google_cloud_go_storage//:storage",
         "@org_golang_google_grpc//codes",

diff --git a/pkg/blobstore/configuration/new_blob_access.go b/pkg/blobstore/configuration/new_blob_access.go
@@ -27,8 +27,6 @@ import (
 	"github.com/buildbarn/bb-storage/pkg/random"
 	"github.com/buildbarn/bb-storage/pkg/util"
 	"github.com/fxtlabs/primes"
-	"github.com/go-redis/redis/extra/redisotel"
-	"github.com/go-redis/redis/v8"
 
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -42,12 +40,6 @@ type BlobAccessInfo struct {
 	DigestKeyFormat digest.KeyFormat
 }
 
-func newRedisClient(opt *redis.Options) *redis.Client {
-	client := redis.NewClient(opt)
-	client.AddHook(redisotel.TracingHook{})
-	return client
-}
-
 func newCachedReadBufferFactory(cacheConfiguration *digest_pb.ExistenceCacheConfiguration, baseReadBufferFactory blobstore.ReadBufferFactory, digestKeyFormat digest.KeyFormat) (blobstore.ReadBufferFactory, error) {
 	if cacheConfiguration == nil {
 		// No caching enabled.
@@ -93,109 +85,6 @@ func (nc *simpleNestedBlobAccessCreator) newNestedBlobAccessBare(configuration *
 			BlobAccess:      readcaching.NewReadCachingBlobAccess(slow.BlobAccess, fast.BlobAccess, replicator),
 			DigestKeyFormat: slow.DigestKeyFormat,
 		}, "read_caching", nil
-	case *pb.BlobAccessConfiguration_Redis:
-		tlsConfig, err := util.NewTLSConfigFromClientConfiguration(backend.Redis.Tls)
-		if err != nil {
-			return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain TLS configuration")
-		}
-
-		var replicationTimeout time.Duration
-		if backend.Redis.ReplicationTimeout != nil {
-			if err := backend.Redis.ReplicationTimeout.CheckValid(); err != nil {
-				return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain replication timeout")
-			}
-			replicationTimeout = backend.Redis.ReplicationTimeout.AsDuration()
-		}
-
-		var dialTimeout time.Duration
-		if backend.Redis.DialTimeout != nil {
-			if err := backend.Redis.DialTimeout.CheckValid(); err != nil {
-				return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain dial timeout configuration")
-			}
-			dialTimeout = backend.Redis.DialTimeout.AsDuration()
-		}
-
-		var readTimeout time.Duration
-		if backend.Redis.ReadTimeout != nil {
-			if err := backend.Redis.ReadTimeout.CheckValid(); err != nil {
-				return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain read timeout configuration")
-			}
-			readTimeout = backend.Redis.ReadTimeout.AsDuration()
-		}
-
-		var writeTimeout time.Duration
-		if backend.Redis.WriteTimeout != nil {
-			if err := backend.Redis.WriteTimeout.CheckValid(); err != nil {
-				return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain write timeout configuration")
-			}
-			writeTimeout = backend.Redis.WriteTimeout.AsDuration()
-		}
-
-		var redisClient blobstore.RedisClient
-		switch mode := backend.Redis.Mode.(type) {
-		case *pb.RedisBlobAccessConfiguration_Clustered:
-			// Gather retry configuration (min/max delay and overall retry attempts)
-			minRetryDur := time.Millisecond * 32
-			if mode.Clustered.MinimumRetryBackoff != nil {
-				if err := mode.Clustered.MinimumRetryBackoff.CheckValid(); err != nil {
-					return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain minimum retry back off configuration")
-				}
-				minRetryDur = mode.Clustered.MinimumRetryBackoff.AsDuration()
-			}
-
-			maxRetryDur := time.Millisecond * 2048
-			if mode.Clustered.MaximumRetryBackoff != nil {
-				if err := mode.Clustered.MaximumRetryBackoff.CheckValid(); err != nil {
-					return BlobAccessInfo{}, "", util.StatusWrap(err, "Failed to obtain maximum retry back off")
-				}
-				maxRetryDur = mode.Clustered.MaximumRetryBackoff.AsDuration()
-			}
-
-			maxRetries := 16 // Default will be 16
-			if mode.Clustered.MaximumRetries != 0 {
-				maxRetries = int(mode.Clustered.MaximumRetries)
-			}
-
-			redisClient = redis.NewClusterClient(
-				&redis.ClusterOptions{
-					Addrs:           mode.Clustered.Endpoints,
-					TLSConfig:       tlsConfig,
-					ReadOnly:        true,
-					MaxRetries:      maxRetries,
-					MinRetryBackoff: minRetryDur,
-					MaxRetryBackoff: maxRetryDur,
-					DialTimeout:     dialTimeout,
-					ReadTimeout:     readTimeout,
-					WriteTimeout:    writeTimeout,
-					NewClient:       newRedisClient,
-				})
-
-		case *pb.RedisBlobAccessConfiguration_Single:
-			redisClient = newRedisClient(
-				&redis.Options{
-					Addr:         mode.Single.Endpoint,
-					Password:     mode.Single.Password,
-					DB:           int(mode.Single.Db),
-					TLSConfig:    tlsConfig,
-					DialTimeout:  dialTimeout,
-					ReadTimeout:  readTimeout,
-					WriteTimeout: writeTimeout,
-				})
-		default:
-			return BlobAccessInfo{}, "", status.Errorf(codes.InvalidArgument, "Redis configuration must either be clustered or single server")
-		}
-
-		digestKeyFormat := creator.GetBaseDigestKeyFormat()
-		return BlobAccessInfo{
-			BlobAccess: blobstore.NewRedisBlobAccess(
-				redisClient,
-				readBufferFactory,
-				digestKeyFormat,
-				backend.Redis.ReplicationCount,
-				replicationTimeout,
-				creator.GetDefaultCapabilitiesProvider()),
-			DigestKeyFormat: digestKeyFormat,
-		}, "redis", nil
 	case *pb.BlobAccessConfiguration_Http:
 		roundTripper, err := bb_http.NewRoundTripperFromConfiguration(backend.Http.Client)
 		if err != nil {
@@ -250,19 +139,6 @@ func (nc *simpleNestedBlobAccessCreator) newNestedBlobAccessBare(configuration *
 				backend.Sharding.HashInitialization),
 			DigestKeyFormat: *combinedDigestKeyFormat,
 		}, "sharding", nil
-	case *pb.BlobAccessConfiguration_SizeDistinguishing:
-		small, err := nc.NewNestedBlobAccess(backend.SizeDistinguishing.Small, creator)
-		if err != nil {
-			return BlobAccessInfo{}, "", err
-		}
-		large, err := nc.NewNestedBlobAccess(backend.SizeDistinguishing.Large, creator)
-		if err != nil {
-			return BlobAccessInfo{}, "", err
-		}
-		return BlobAccessInfo{
-			BlobAccess:      blobstore.NewSizeDistinguishingBlobAccess(small.BlobAccess, large.BlobAccess, backend.SizeDistinguishing.CutoffSizeBytes),
-			DigestKeyFormat: small.DigestKeyFormat.Combine(large.DigestKeyFormat),
-		}, "size_distinguishing", nil
 	case *pb.BlobAccessConfiguration_Mirrored:
 		backendA, err := nc.NewNestedBlobAccess(backend.Mirrored.BackendA, creator)
 		if err != nil {

diff --git a/pkg/blobstore/redis_blob_access.go b/pkg/blobstore/redis_blob_access.go