TrueCloudLab/frostfs-sdk-go
33
0
Fork 21
Code Issues 34 Pull requests 2 Releases Activity Actions

Add buffer support for payloadSizeLimiter #197

Merged
fyrchik merged 1 commit from achuprov/frostfs-sdk-go:buff_pool into master 2024-03-27 08:53:31 +00:00
Conversation 1 Commits 1 Files changed 3 +24 -1
3 changed files with 24 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 1af9b6d18b - Show all commits

3
client/object_put.go
View file

@ -4,6 +4,7 @@ import (
"context" "context"
"crypto/ecdsa" "crypto/ecdsa"
buffPool "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/util/pool"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
@ -36,6 +37,8 @@ type PrmObjectPutInit struct {
WithoutHomomorphHash bool WithoutHomomorphHash bool
Key *ecdsa.PrivateKey Key *ecdsa.PrivateKey
Pool *buffPool.BufferPool
dkirillov commented 2024-02-02 06:12:56 +00:00
Outdated
Review
Copy link

Maybe it's better to change if the file is large -> if the object is large?

Maybe it's better to change `if the file is large` -> `if the object is large`?
fyrchik commented 2024-02-05 08:45:50 +00:00
Outdated
Review
Copy link

We write that it is optional, but do not say what it is and why we may need this.
Can we extend the description and use appropriate godoc format? (// Pool is ...)
Also, other optional fields do not have comment, not sure we need it.

We write that it is optional, but do not say what it is and why we may need this. Can we extend the description and use appropriate godoc format? (`// Pool is ...`) Also, other optional fields do not have comment, not sure we need it.
achuprov commented 2024-02-09 10:31:46 +00:00
Outdated
Review
Copy link

Fixed

Fixed
} }
// SetCopiesNumber sets number of object copies that is enough to consider put successful. // SetCopiesNumber sets number of object copies that is enough to consider put successful.

12
client/object_put_transformer.go
View file

@ -3,6 +3,7 @@ package client
import ( import (
"context" "context"
buffPool "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/util/pool"
apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/transformer" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/transformer"
@ -26,6 +27,7 @@ func (c *Client) objectPutInitTransformer(prm PrmObjectPutInit) (*objectWriterTr
MaxSize: prm.MaxSize, MaxSize: prm.MaxSize,
WithoutHomomorphicHash: prm.WithoutHomomorphHash, WithoutHomomorphicHash: prm.WithoutHomomorphHash,
NetworkState: prm.EpochSource, NetworkState: prm.EpochSource,
Pool: prm.Pool,
}) })
return &w, nil return &w, nil
} }
@ -114,6 +116,7 @@ func (it *internalTarget) tryPutSingle(ctx context.Context, o *object.Object) (b
} }
if err == nil { if err == nil {
it.returnBuffPool(o.Payload())
id, _ := o.ID() id, _ := o.ID()
it.res = &ResObjectPut{ it.res = &ResObjectPut{
statusRes: res.statusRes, statusRes: res.statusRes,
@ -126,3 +129,12 @@ func (it *internalTarget) tryPutSingle(ctx context.Context, o *object.Object) (b
} }
return true, err return true, err
} }
func (it *internalTarget) returnBuffPool(playback []byte) {
if it.prm.Pool == nil {
return
}
var buffer buffPool.Buffer
buffer.Data = playback
it.prm.Pool.Put(&buffer)
}

10
object/transformer/transformer.go
View file

@ -6,6 +6,7 @@ import (
"crypto/sha256" "crypto/sha256"
"fmt" "fmt"
buffPool "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/util/pool"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/checksum" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/checksum"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
@ -45,6 +46,7 @@ type Params struct {
// functionality. Primary usecases are providing file size when putting an object // functionality. Primary usecases are providing file size when putting an object
// with the frostfs-cli or using Content-Length header in gateways. // with the frostfs-cli or using Content-Length header in gateways.
SizeHint uint64 SizeHint uint64
Pool *buffPool.BufferPool
} }
// NewPayloadSizeLimiter returns ObjectTarget instance that restricts payload length // NewPayloadSizeLimiter returns ObjectTarget instance that restricts payload length
@ -137,7 +139,13 @@ func (s *payloadSizeLimiter) initializeCurrent() {
payloadSize = remaining % s.MaxSize payloadSize = remaining % s.MaxSize
} }
} }
s.payload = make([]byte, 0, payloadSize)
if s.Pool == nil {
s.payload = make([]byte, 0, payloadSize)
} else {
buffer := s.Pool.Get(uint32(payloadSize))
s.payload = buffer.Data[:0]
fyrchik commented 2024-01-25 12:09:41 +00:00
Outdated
Review
Copy link

So we will allocate 64 MiB even for 1KiB object?
There is a benchmark in the transformer package btw, could be useful.

So we will allocate 64 MiB even for 1KiB object? There is a benchmark in the transformer package btw, could be useful.
fyrchik commented 2024-02-09 14:02:20 +00:00
Outdated
Review
Copy link

@achuprov the question is still valid

@achuprov the question is still valid
achuprov commented 2024-03-12 18:14:38 +00:00
Outdated
Review
Copy link

fixed

fixed
}
} }
func (s *payloadSizeLimiter) initPayloadHashers() { func (s *payloadSizeLimiter) initPayloadHashers() {