client: Add ReplicateObject method
#535
Conversation
cthulhu-rider
commented
Dec 14, 2023
cthulhu-rider
commented
- inspired by Replication without decoding neofs-node#2317
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
from
de1242a to
a4a8dab
Compare
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
3 times, most recently
from
67afdd0 to
814b9da
Compare
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
from
814b9da to
e9d8491
Compare
cthulhu-rider
requested review from
roman-khimov,
smallhive and
carpawell
as code owners
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
from
e9d8491 to
3524e7f
Compare
client/object_put.go
Outdated
| // replicated object; | ||
| // - [apistatus.ErrContainerNotFound]: the container to which the replicated | ||
| // object is associated was not found. | ||
| func (c *Client) ReplicateObject(ctx context.Context, src io.ReadSeeker, signer neofscrypto.Signer) error { |
There was a problem hiding this comment.
-1 for building objects in SDK. that is what api-go is created for to me
There was a problem hiding this comment.
building objects
u mean messages (cuz NeoFS objects are not "built" within this scope)? api-go lib doesnt provide functionality for this, so i implemented it in-place
There was a problem hiding this comment.
In some ways you have to replicate things from api-go here from what I see (constants and alike). At the same time api-go itself doesn't seem to be valuable, no ones uses except for SDK and no one will.
There was a problem hiding this comment.
u mean messages (cuz NeoFS objects are not "built" within this scope)?
yes. the same way SDK does not have any Marshal() func implementation. SDK's go.mod does not have any explicit proto/grpc lib. why now? because of some new regular RPC?
api-go lib doesnt provide functionality for this
extend api-go then?
There was a problem hiding this comment.
SDK's go.mod does not have any explicit proto/grpc lib
this does not depend on this PR, anyway these packages are required ( // indirect remark doesn't mean that much)
extend api-go then?
this should be motivated (and can be done at any time btw). I agree with @roman-khimov that right now no ones uses except for SDK and no one will
There was a problem hiding this comment.
this does not depend on this PR
this does depend on this PR cause it adds them
// indirect remark doesn't mean that much
it means a lot. it explicitly says that this repo does not use it and there is no code about it. SDK uses API, API uses proto/grpc. now SDK uses grpc
this should be motivated
proto things should be done in a proto repo? not enough?
I agree with @roman-khimov that right now no ones uses except for SDK and no one will
how does it relate to my comment? we can have ObjectReplicateInit in SDK and ReplicateObject in API the same way how it has been done for every method before. if we need some additional helper func that can build messages based on raw bytes, then we can add them in the API, no problem. how has a new RPC become a reason for a cross-repo movement?
client/object_put.go
Outdated
| // replicated object; | ||
| // - [apistatus.ErrContainerNotFound]: the container to which the replicated | ||
| // object is associated was not found. | ||
| func (c *Client) ReplicateObject(ctx context.Context, src io.ReadSeeker, signer neofscrypto.Signer) error { |
There was a problem hiding this comment.
In some ways you have to replicate things from api-go here from what I see (constants and alike). At the same time api-go itself doesn't seem to be valuable, no ones uses except for SDK and no one will.
client/object_put.go
Outdated
| protowire.SizeTag(fieldNumSignature) + sigSize | ||
|
|
||
| // TODO(#544): support external buffers | ||
| msg := make([]byte, 0, uint64(msgSize)+objSize) |
There was a problem hiding this comment.
if it's related to ur previous comment, then yes: scary and dummy, but that's what is done within Put anyway. So, i just opened wounds. TODO above suggests thinking about it
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
3 times, most recently
from
dfa5b6b to
f1cfcca
Compare
NeoFS storage nodes replicate objects between each other to follow the objects' storage policies. The nodes store objects in Protocol Buffers V3 binary format. Previously, to transmit them, there was a need to almost completely decode the object. NeoFS API V2 protocol was recently extended with `neo.fs.v2.object.ObjectService.Replicate` RPC that allows to transmit binary object in one message (unlike complex `Put` streaming RPC). SDK should provide API for new service RPC clients. Add `ReplicateObject` method that works with binary objects accessed through `io.ReadSeeker` stream. The method is optimized to allocate buffer for the whole object once. Signed-off-by: Leonard Lyubich <[email protected]>
Sometimes it may be necessary to replicate an object across multiple nodes, i.e. using multiple client connections. Normal use of `ReplicateObject` method will result in unrelated message encodings that actually match when the object is fixed. An obvious improvement is message deduplication. This is achieved by wrapping the incoming binary object stream in a synchronous context. The request message is encoded once and then only read. Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
from
f1cfcca to
2b1e5e2
Compare
