-
Notifications
You must be signed in to change notification settings - Fork 139
Add a generic message reader interface #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
meroton-benjamin
wants to merge
5
commits into
buildbarn:main
Choose a base branch
from
meroton:pr/message-reader
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6007c1a
Add a generic message reader interface
meroton-benjamin fd8a94c
Remove non cas use of message reader
meroton-benjamin adae41f
Move MessageReader to pkg/cas
meroton-benjamin 25fe429
Remove storage message from MessageReader signature
meroton-benjamin 32125a7
Use message reader in bb-copy
meroton-benjamin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package blobstore | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/buildbarn/bb-storage/pkg/cas" | ||
| "github.com/buildbarn/bb-storage/pkg/digest" | ||
| "google.golang.org/protobuf/proto" | ||
| ) | ||
|
|
||
| type blobAccessMessageReader[T proto.Message] struct { | ||
| blobAccess BlobAccess | ||
| maximumMessageSizeBytes int | ||
| allocator func() T | ||
| } | ||
|
|
||
| // NewBlobAccessMessageReader creates a storage.MessageReader that reads | ||
| // a message from a BlobAccess up to maximumMessageSizeBytes large. | ||
| // Requires an allocator that returns allocated objects of type T. | ||
| func NewBlobAccessMessageReader[T proto.Message](blobAccess BlobAccess, maximumMessageSizeBytes int, allocator func() T) cas.MessageReader[T] { | ||
| return blobAccessMessageReader[T]{ | ||
| blobAccess: blobAccess, | ||
| maximumMessageSizeBytes: maximumMessageSizeBytes, | ||
| allocator: allocator, | ||
| } | ||
| } | ||
|
|
||
| func (mr blobAccessMessageReader[T]) ReadMessage(ctx context.Context, d digest.Digest) (T, error) { | ||
| var zero T | ||
| ret, err := mr.blobAccess.Get(ctx, d).ToProto(mr.allocator(), mr.maximumMessageSizeBytes) | ||
| if err != nil { | ||
| return zero, err | ||
| } | ||
| return ret.(T), nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,25 +8,40 @@ import ( | |
| remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" | ||
| "github.com/buildbarn/bb-storage/pkg/blobstore" | ||
| "github.com/buildbarn/bb-storage/pkg/blobstore/buffer" | ||
| "github.com/buildbarn/bb-storage/pkg/cas" | ||
| "github.com/buildbarn/bb-storage/pkg/digest" | ||
| "github.com/buildbarn/bb-storage/pkg/util" | ||
|
|
||
| "google.golang.org/protobuf/encoding/protowire" | ||
| ) | ||
|
|
||
| // CASReplicator replicates blobs to the destination Content | ||
| // Addressable Storage. | ||
| type CASReplicator interface { | ||
| ReplicateMultiple(ctx context.Context, digests digest.Set) error | ||
| } | ||
|
|
||
| // BlobStreamReader provides a stream to the raw bytes of a blob. | ||
| type BlobStreamReader interface { | ||
| ReadStream(ctx context.Context, d digest.Digest) (io.ReadCloser, error) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just thinking: aren't we going to need an interface like this on the NaiveBuildDirectory side to download files? If so, maybe this should also live in |
||
| } | ||
|
|
||
| type blobToReplicate struct { | ||
| digest digest.Digest | ||
| expanderFunc func(ctx context.Context, b buffer.Buffer) error | ||
| expanderFunc func(ctx context.Context, d digest.Digest) error | ||
| } | ||
|
|
||
| // NestedBlobReplicator is a helper type for BlobReplicator that can be | ||
| // used to copy nested hierarchies of objects stored in the Content | ||
| // Addressable Storage (CAS). In the case of the REv2 protocol, these | ||
| // are Action, Directory and Tree messages. | ||
| type NestedBlobReplicator struct { | ||
| replicator BlobReplicator | ||
| replicator CASReplicator | ||
| digestKeyFormat digest.KeyFormat | ||
| maximumMessageSizeBytes int | ||
| actionReader cas.MessageReader[*remoteexecution.Action] | ||
| directoryReader cas.MessageReader[*remoteexecution.Directory] | ||
| treeReader BlobStreamReader | ||
|
|
||
| lock sync.Mutex | ||
| blobsSeen map[string]struct{} | ||
|
|
@@ -37,17 +52,20 @@ type NestedBlobReplicator struct { | |
|
|
||
| // NewNestedBlobReplicator creates a new NestedBlobReplicator that does | ||
| // not have any objects to be replicated queued. | ||
| func NewNestedBlobReplicator(replicator BlobReplicator, digestKeyFormat digest.KeyFormat, maximumMessageSizeBytes int) *NestedBlobReplicator { | ||
| func NewNestedBlobReplicator(replicator CASReplicator, maximumMessageSizeBytes int, actionReader cas.MessageReader[*remoteexecution.Action], directoryReader cas.MessageReader[*remoteexecution.Directory], treeReader BlobStreamReader, digestKeyFormat digest.KeyFormat) *NestedBlobReplicator { | ||
| return &NestedBlobReplicator{ | ||
| replicator: replicator, | ||
| digestKeyFormat: digestKeyFormat, | ||
| maximumMessageSizeBytes: maximumMessageSizeBytes, | ||
| digestKeyFormat: digestKeyFormat, | ||
| actionReader: actionReader, | ||
| directoryReader: directoryReader, | ||
| treeReader: treeReader, | ||
|
|
||
| blobsSeen: map[string]struct{}{}, | ||
| } | ||
| } | ||
|
|
||
| func (nr *NestedBlobReplicator) enqueue(blobDigest digest.Digest, expanderFunc func(ctx context.Context, b buffer.Buffer) error) { | ||
| func (nr *NestedBlobReplicator) enqueue(blobDigest digest.Digest, expanderFunc func(ctx context.Context, d digest.Digest) error) { | ||
| nr.lock.Lock() | ||
| defer nr.lock.Unlock() | ||
|
|
||
|
|
@@ -73,12 +91,11 @@ func (nr *NestedBlobReplicator) maybeWakeUpLocked() { | |
| // referenced input root and Command message will be replicated as well. | ||
| func (nr *NestedBlobReplicator) EnqueueAction(actionDigest digest.Digest) { | ||
| digestFunction := actionDigest.GetDigestFunction() | ||
| nr.enqueue(actionDigest, func(ctx context.Context, b buffer.Buffer) error { | ||
| actionMessage, err := b.ToProto(&remoteexecution.Action{}, nr.maximumMessageSizeBytes) | ||
| nr.enqueue(actionDigest, func(ctx context.Context, d digest.Digest) error { | ||
| action, err := nr.actionReader.ReadMessage(ctx, d) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| action := actionMessage.(*remoteexecution.Action) | ||
|
|
||
| inputRootDigest, err := digestFunction.NewDigestFromProto(action.InputRootDigest) | ||
| if err != nil { | ||
|
|
@@ -102,12 +119,11 @@ func (nr *NestedBlobReplicator) EnqueueAction(actionDigest digest.Digest) { | |
| // well, recursively. | ||
| func (nr *NestedBlobReplicator) EnqueueDirectory(directoryDigest digest.Digest) { | ||
| digestFunction := directoryDigest.GetDigestFunction() | ||
| nr.enqueue(directoryDigest, func(ctx context.Context, b buffer.Buffer) error { | ||
| directoryMessage, err := b.ToProto(&remoteexecution.Directory{}, nr.maximumMessageSizeBytes) | ||
| nr.enqueue(directoryDigest, func(ctx context.Context, d digest.Digest) error { | ||
| directory, err := nr.directoryReader.ReadMessage(ctx, d) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| directory := directoryMessage.(*remoteexecution.Directory) | ||
|
|
||
| for i, childDirectory := range directory.Directories { | ||
| childDigest, err := digestFunction.NewDigestFromProto(childDirectory.Digest) | ||
|
|
@@ -136,8 +152,11 @@ func (nr *NestedBlobReplicator) EnqueueDirectory(directoryDigest digest.Digest) | |
| // file will be replicated as well. | ||
| func (nr *NestedBlobReplicator) EnqueueTree(treeDigest digest.Digest) { | ||
| digestFunction := treeDigest.GetDigestFunction() | ||
| nr.enqueue(treeDigest, func(ctx context.Context, b buffer.Buffer) error { | ||
| r := b.ToReader() | ||
| nr.enqueue(treeDigest, func(ctx context.Context, d digest.Digest) error { | ||
| r, err := nr.treeReader.ReadStream(ctx, d) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer r.Close() | ||
|
|
||
| // Gather digests of files contained in the directories. | ||
|
|
@@ -214,9 +233,10 @@ func (nr *NestedBlobReplicator) Replicate(ctx context.Context) error { | |
| // Replicate a single object. | ||
| nr.blobsReplicating++ | ||
| nr.lock.Unlock() | ||
| nr.replicator.ReplicateMultiple(ctx, blobToReplicate.digest.ToSingletonSet()) | ||
| err := blobToReplicate.expanderFunc( | ||
| ctx, | ||
| nr.replicator.ReplicateSingle(ctx, blobToReplicate.digest), | ||
| blobToReplicate.digest, | ||
| ) | ||
| nr.lock.Lock() | ||
| nr.blobsReplicating-- | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you change this line to:
Then you can use
Tto refer to a concrete message type, andTPtrwhenever you need a pointer to one. That way you can get rid of the allocator function.