Allow to split objects in the client #94
No reviewers
Labels
No labels
P0
P1
P2
P3
good first issue
pool
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-sdk-go#94
Loading…
Reference in a new issue
No description provided.
Delete branch "acid-ant/frostfs-sdk-go:feature/83-cut-obj-client"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Close #83
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com
@ -0,0 +21,4 @@
w.ctx, cancel = context.WithCancel(ctx)
stream, err := rpcapi.PutObject(&c.c, &w.respV2, client.WithContext(w.ctx))
if err != nil {
cancel()
I know that is from previos
ObjectPutInit
implementation. But I was thinking: is it really necessary to invokecancel()
from inherited context whenPutObject
has already returned the error andw.ctx
is not passed somewhere withinobjectPutInitRaw
func?Updated, please review.
@ -0,0 +57,4 @@
req v2object.PutRequest
partInit v2object.PutObjectPartInit
partChunk v2object.PutObjectPartChunk
ctx context.Context
Looks unused
Removed.
@ -0,0 +37,4 @@
type objectWriterTransformer struct {
ot transformer.ObjectTarget
it internalTarget
ctx context.Context
I think ctx should be passed as an argument
Done.
@ -0,0 +28,4 @@
Key: key,
NextTargetInit: func() transformer.ObjectTarget { return &w.it },
MaxSize: prm.maxSize,
WithoutHomomorphicHash: true,
Shouldn't it be parameterized? We didn't abandon homomorphic hashes as far as I know.
Definitely should, I've updated pr.
6f8f73317d
to9003259522
@ -23,0 +18,4 @@
// we simply assume that 3MB is large enough to reduce the
// number of messages, and not to exceed the limit
// (4MB by default for gRPC servers).
const MaxChunkLen = 3 << 20
Why is it exported?
The idea is to use it here, for example https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/cmd/frostfs-cli/internal/client/client.go#L416
@ -77,0 +62,4 @@
type ObjectWriter interface {
// WriteHeader writes header of the object. Result means success.
// Failure reason can be received via Close.
WriteHeader(object.Object) bool
If we change it, may be use
context.Context
in signatures?Updated, please review.
@ -237,1 +130,3 @@
return &x.res, nil
// WithEpoch specifies epoch for object when cut it on client side.
func (x *PrmObjectPutInit) WithEpoch(epoch uint64) {
x.epoch = epoch
I think the parameter should not be obligatory, we could try
netmap.Snapshot
isepoch == 0
.Updated, please review.
9003259522
to73f9f4d4f4
Passing
context.Context
as argument forWrite.../Close
functions.Allow to cut objects in the clientto Allow to split objects in the client73f9f4d4f4
to9e3f0c0017
acid-ant referenced this pull request from TrueCloudLab/frostfs-node2023-06-27 06:33:58 +00:00
acid-ant referenced this pull request from TrueCloudLab/xk6-frostfs2023-06-27 06:54:49 +00:00
@ -23,0 +19,4 @@
// we simply assume that 3MB is large enough to reduce the
// number of messages, and not to exceed the limit
// (4MB by default for gRPC servers).
const MaxChunkLen = 3 << 20
Is is needed after #96?
Think yes, just to avoid using some internal values from sdk across the code base in node.
Ok, I guess we forgot to rename private field accordingly.
Can we name it similar to
SetGRPCPayloadChunkLen
, likeDefaultGRPCPayloadChunkLen
?Agree, I've renamed and reverted changes.
@ -0,0 +46,4 @@
func (x *objectWriterTransformer) Close(ctx context.Context) (*ResObjectPut, error) {
if ai, err := x.ot.Close(ctx); err != nil {
return nil, err
} else {
This
else
is redundant (I wonder why linter does not complain)The idea is to reduce the number of lines here. To remove
else
we need to defineai
outsideif
.9e3f0c0017
to2a40c47398
Resolved conflicts.
2a40c47398
to5df468f6e3
@ -89,0 +72,4 @@
WriteHeader(context.Context, object.Object) bool
// WritePayloadChunk writes chunk of the object payload. Result means success.
// Failure reason can be received via Close.
WritePayloadChunk(context.Context, []byte) bool
As far as I understand, this interface hides object transformation details and should be used as a replacement for current payload streaming approach, with the difference that object is created on the client side.
So
objectWriter
implementation is not suitable for more complex cases when client application wants to resend the object in case of transmission failures. Should client application use transformers directly in the code to do that and then use this interface to upload and reupload created object?Thought about this, too. There is no mechanism to resend part of the already formed object. Currently, implemented happy path only. So client should
transformer
directly.Do you think we need to add some retries when sending parts of the big object?
Probably no. But client library will benefit from having an interface to put already prepared object, in my opinion.
You talk about a new method
WriteObject
which should wrapWriteHeader
andWritePayloadChunk
?And it looks like raw writer only should implement it.
Oh yes. I see there is an separate issue for that #64, so we can postpone it. Also we are going to have one more RPC to upload whole object in one request: TrueCloudLab/frostfs-api#9.