Add support for SplitBlob and SpliceBlob methods by meroton-benjamin · Pull Request #345 · buildbarn/bb-storage

meroton-benjamin · 2026-05-31T20:23:38Z

This commit adds support for the SplitBlob and SpliceBlob methods from the Remote Execution v2 (REv2) api. SplitBlob and SpliceBlob can be used to facilitate uploads and downloads of large files but a naïve implementation like this has some major drawbacks as well.

The blobs must exist in both their chunked and non chunked form, which may significantly increase storage requirements for large blobs. The protocol gives no guarantee that a large blob stored in the CAS exists in its chunked form which forces you to perform a fairly heavy Split call that loads the entire large blob in order to decomposition it into its chunks.

This implementation mostly exists as a stepping stone for a different implementation where Buildbarn internally manages all blobs as chunked blobs.

EdSchouten · 2026-06-01T08:27:58Z

-func (contentAddressableStorageServer) SpliceBlob(ctx context.Context, in *remoteexecution.SpliceBlobRequest) (*remoteexecution.SpliceBlobResponse, error) {
-	return nil, status.Error(codes.Unimplemented, "This service does not support splicing blobs")
+func (s *contentAddressableStorageServer) SpliceBlob(ctx context.Context, in *remoteexecution.SpliceBlobRequest) (*remoteexecution.SpliceBlobResponse, error) {
+	if s.chunkListStorage == nil {


Instead of doing this, why don't we require that ChunkListStorage is non-nil, and that an ErrorBlobAccess is passed in?

We could but then the ContentAddressableStorageServer would be unable to behave differently when the ChunkListStorage is not defined. If we want the FindMissingBlobs implementation from below then we need to know when we need to verify chunk list storage and when we do not.

Maybe good to at least add a // TODO: Require that s.chunkListStorage is non-null once we require chunking. ?

EdSchouten · 2026-06-01T08:39:52Z

+
+// fakeBlobAccess provides a thread-safe, in-memory BlobAccess for
+// testing.
+type fakeBlobAccess struct {


Can't we just use gomock for consistency with most of the other tests in Buildbarn?

Eh we sort of could but it becomes noisy, for some of the tests a straight up EXPECT().Return() construct flows well but for the more complicated tests such as the ones where we want to verify chunking and that files have been properly touched it becomes very noisy.

For those tests we could use .EXPECT().DoAndReturn().AnyTimes() to implement pretty much the same thing as in fakeBlobAccess already does but that would basically be the same thing as fakeBlobAccess just with lambdas instead of functions.

If you prefer that anyway I can modify the code such that it uses gomock for consistency but at least in my mind fakeBlobAccess was easier to understand.

Do you have any input on if we want to turn fakeBlobAccess into a lambda construct with gomock for consistency or if it's fine as is?

EdSchouten · 2026-07-14T18:43:32Z

+				grpcClientFactory,
+			)
+			if err != nil {
+				return util.StatusWrap(err, "Failed to create Chunk Map")


Failed to create Chunk List Storage?

EdSchouten · 2026-07-14T18:48:42Z

-func (contentAddressableStorageServer) SpliceBlob(ctx context.Context, in *remoteexecution.SpliceBlobRequest) (*remoteexecution.SpliceBlobResponse, error) {
-	return nil, status.Error(codes.Unimplemented, "This service does not support splicing blobs")
+func (s *contentAddressableStorageServer) SpliceBlob(ctx context.Context, in *remoteexecution.SpliceBlobRequest) (*remoteexecution.SpliceBlobResponse, error) {
+	if s.chunkListStorage == nil {


Maybe good to at least add a // TODO: Require that s.chunkListStorage is non-null once we require chunking. ?

EdSchouten · 2026-07-14T18:50:21Z

+				bigBlobDigests.Add(digest)
+			}
+		}
+		_, _ = s.chunkListStorage.FindMissing(ctx, bigBlobDigests.Build())


Sure we don't need any error/results handling here...?

We perform the call towards the chunk list storage in order to prevent the chunk list storage itself from being dropped from the cache if they exist. As we've yet to make the chunking mandatory semantically the blob is not missing even if it's chunk list is missing.

With regards to errors, I'm not sure how we would treat errors at this point.

EdSchouten · 2026-07-14T18:52:50Z

+	switch backend := configuration.Backend.(type) {
+	case *pb.BlobAccessConfiguration_ChunkListValidating:
+		if bac.contentAddressableStorage == nil {
+			return BlobAccessInfo{}, "", status.Error(codes.InvalidArgument, "Action Cache completeness checking can only be enabled if a Content Addressable Storage is configured")


s/Action Cache completeness checking/.../ ?

Replaced with "Chunk list validation can only be enabled if a Content Addressable Storage is configured"

EdSchouten · 2026-07-14T18:55:19Z

+		return nil, status.Error(codes.Unimplemented, "This backend only supports upstream servers with rep max cdc support.")
+	}
+	if params.MinChunkSizeBytes < 64 {
+		return nil, status.Errorf(codes.Internal, "MinChunkSizeBytes was %d but a minimum of 64 is required.", params.MinChunkSizeBytes)


"RepMaxCDC minimum chunk size was %d bytes, but a minimum of 64 bytes is required" ?

EdSchouten · 2026-07-14T18:56:30Z

+		return nil, util.StatusWrap(err, "Unable to GetCapabilities to determine chunking parameters")
+	}
+
+	params := capabilities.GetCacheCapabilities().GetRepMaxCdcParams()


s/GetCacheCapabilities()/CacheCapabilities/

In the Buildbarn tree I only try to use .Get*() in cases where the parent may be nil. That way it's sort of self-documenting what can be nil or not. In this case the top-level message is always non-nil.

EdSchouten · 2026-07-14T18:57:02Z

+	}
+	maxMinChunkSize := (ba.maximumMessageSizeBytes + 1) / 2
+	if params.MinChunkSizeBytes > uint64(maxMinChunkSize) {
+		return nil, status.Errorf(codes.Internal, "MinChunkSizeBytes was %d but a maximum of %d is supported with the configured maximum message size.", params.MinChunkSizeBytes, maxMinChunkSize)


Please make sure to remove trailing dots from the error messages.

This commit adds support for the SplitBlob and SpliceBlob methods from the Remote Execution v2 (REv2) api. SplitBlob and SpliceBlob can be used to facilitate uploads and downloads of large files but a naïve implementation like this has some major drawbacks as well. The blobs must exist in both their chunked and non chunked form, which may significantly increase storage requirements for large blobs. The protocol gives no guarantee that a large blob stored in the CAS exists in its chunked form which forces you to perform a fairly heavy Split call that loads the entire large blob in order to decomposition it into its chunks. This implementation mostly exists as a stepping stone for a different implementation where Buildbarn internally manages all blobs as chunked blobs.

meroton-benjamin force-pushed the cdc-support-step-1 branch from 17fa168 to 5ef05b3 Compare June 1, 2026 08:16

EdSchouten reviewed Jun 1, 2026

View reviewed changes

meroton-benjamin force-pushed the cdc-support-step-1 branch 7 times, most recently from c20835c to e8f71eb Compare June 3, 2026 11:42

moroten force-pushed the cdc-support-step-1 branch from 343d61a to 9c637a9 Compare June 5, 2026 06:21

meroton-benjamin force-pushed the cdc-support-step-1 branch from 9c637a9 to 06da40e Compare June 22, 2026 05:36

EdSchouten reviewed Jul 14, 2026

View reviewed changes

meroton-benjamin force-pushed the cdc-support-step-1 branch from 06da40e to 0a33eb5 Compare July 15, 2026 16:20

meroton-benjamin force-pushed the cdc-support-step-1 branch from 0a33eb5 to 190edf7 Compare August 17, 2026 15:19

meroton-benjamin force-pushed the cdc-support-step-1 branch from 190edf7 to d5f83fb Compare August 17, 2026 15:20

Uh oh!

Conversation

meroton-benjamin commented May 31, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants