object: Separate API for node-to-node replication #280
Conversation
object/service.proto
Outdated
| // TODO: docs | ||
| message ReplicationRequest { | ||
| // TODO: reserve 1st byte for forward compatibility | ||
| bytes object = 1; |
There was a problem hiding this comment.
Can we have it be structured then? Like a proper Object? That's the one we send anyway, so if you have it serialized, that's your message, or not?
There was a problem hiding this comment.
yeah all messages are binary
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
3 times, most recently
from
597124f
to
bd1ffaf
Compare
|
@roman-khimov ive finalized format and docs, take a look pls |
object/service.proto
Outdated
| // Replicate RPC response | ||
| message ReplicateResponse { | ||
| // Operation status codes. | ||
| enum Code { |
There was a problem hiding this comment.
No ResponseMetaHeader? No ResponseVerificationHeader? No common codes? I'd expect it at least to follow common status data and reuse existing codes (ResponseMetaHeader). Separate codes can also be very inconvenient.
There was a problem hiding this comment.
predictable questions. For now, I only doubt the need to add a signature similar to the one in the request. The rest is overhead for this simple operation
There was a problem hiding this comment.
Unified error codes are very valuable to me.
There was a problem hiding this comment.
will reuse, not yet supported cases can fallback to INTERNAL (like #269)
There was a problem hiding this comment.
We can extend the set if needed.
There was a problem hiding this comment.
there is no urgent need now
Previously, object replication between container nodes was performed via `ObjectService.Put` RPC. This approach had a number of problems: - system operations inside the container were mixed with user requests - it was not possible to send an object in one message To improve this, a separate `Replicate` RPC is added. It's intended for usage by nodes of the containers only to comply with their storage policies. Within a request, any object is packaged into one message. Only the object itself is signed, which simplifies and speeds up the formation and processing of the request. Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider
force-pushed
the
feature/object-separate-replication
branch
from
bd1ffaf
to
e2f4b50
Compare
| // Replicate RPC response | ||
| message ReplicateResponse { | ||
| // Operation execution status with one of the enumerated codes. | ||
| neo.fs.v2.status.Status status = 1; |
There was a problem hiding this comment.
I like the simplicity of it, but this breaks https://github.com/nspcc-dev/neofs-sdk-go/blob/master/client/response.go and associated code around it. Does it affect integration?
There was a problem hiding this comment.
nope, integration is ok cuz this is purely new functionality. If protocol doesnt require signature (if we keep the current status-only response), then applications must not too
the only tricky thing i see is #201 (comment), but there are app-side workarounds
the main thing for us is to think carefully about whether we need a signature from the protocol pov. And ofc, when in doubt, it is better to add it and then make it optional, because further additions must be optional for the backward compat sake. But, at the same time, the benefits are questionable while the resource costs are real
im leaning more towards the option without a signature as more efficient, but I'm not completely sure yet. Share ur thoughts pls @roman-khimov @carpawell
There was a problem hiding this comment.
Signatures make some sense for insecure connections or for rerouted messages, but for TLS-enabled 1:1 calls they're irrelevant. And then there is also some benefit in having insecure unsigned option for fast communication over known good channels. So if this inconsistency is not an obstacle, we better go without signatures.
Now for the other data like epochs, is it important for us to get it with every reply? I doubt that, but maybe I'm missing something. This data costs nothing to transmit/receive.
There was a problem hiding this comment.
same thoughts about transport security. Can we spread this to requests also? In proposed changes, replication requests are signed while responses are not
to get it with every reply
absolutely not. For me, this is redundancy useful for a narrow set of tasks (mostly system ones), which should not be unconditionally transferred. I see the behavior: if client set flag in the request, epoch/smth will be returned, not by default. Although the amount of data like epoch is insignificant, it is still pointless to transfer it when not needed by the client
There was a problem hiding this comment.
i think a signature absence is acceptable for a system-only RPC response
|
|
||
| // Replicate RPC response | ||
| message ReplicateResponse { | ||
| // Operation execution status with one of the enumerated codes. |
There was a problem hiding this comment.
i would say it is an incorrect comment now. i see Status, no enumeration
| // Replicate RPC response | ||
| message ReplicateResponse { | ||
| // Operation execution status with one of the enumerated codes. | ||
| neo.fs.v2.status.Status status = 1; |
There was a problem hiding this comment.
i think a signature absence is acceptable for a system-only RPC response
here is a design of the separate object replication API
in addition to general simplification, the main motivation is to reduce amount of transmitted service data (request headers, signatures, stream markup, etc.) compared to general purpose
rpc Put