[#559] Remove multipart objects using tombstones #560
No reviewers
Labels
No labels
P0
P1
P2
P3
good first issue
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-s3-gw#560
Loading…
Reference in a new issue
No description provided.
Delete branch "mbiryukova/frostfs-s3-gw:feature/multipart_tombstone"
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?
Closes #559
Signed-off-by: Marina Biryukova m.biryukova@yadro.com
@ -636,3 +636,3 @@
}
if err = h.obj.AbortMultipartUpload(ctx, p); err != nil {
networkInfo, err := h.obj.GetNetworkInfo(ctx)
It seems we can get network info inside
h.obj.AbortMultipartUpload
handler@ -218,2 +226,4 @@
AttributeFrostfsCopiesNumber = "frostfs-copies-number" // such format to match X-Amz-Meta-Frostfs-Copies-Number header
tombstoneLifetime = uint64(10)
Can we make this configurable?
@ -772,0 +779,4 @@
var wg sync.WaitGroup
i := 0
tombstoneMembersSize := n.features.TombstoneMembersSize()
for ; i < len(members)/tombstoneMembersSize; i++ {
What about something like this:
@ -772,0 +809,4 @@
}
}
func (n *Layer) getMembers(ctx context.Context, bkt *data.BucketInfo, objID oid.ID) []oid.ID {
Did you consider using
relations.Relations
from sdk?Something like this:
@ -82,6 +82,18 @@ func (n *Layer) objectHead(ctx context.Context, bktInfo *data.BucketInfo, idObj
return n.frostFS.HeadObject(ctx, prm)
}
func (n *Layer) objectHeadRaw(ctx context.Context, bktInfo *data.BucketInfo, idObj oid.ID) (*object.Object, error) {
What about
@ -239,6 +251,7 @@ func newAppSettings(log *Logger, v *viper.Viper) *appSettings {
reconnectInterval: fetchReconnectInterval(v),
frostfsidValidation: v.GetBool(cfgFrostfsIDValidationEnabled),
dialerSource: getDialerSource(log.logger, v),
workerPoolSize: v.GetInt(cfgWorkerPoolSize),
Can we use
fetch...
function that check config provided value and will return default if it's 0 or less?@ -228,1 +228,4 @@
source_ip_header: "Source-Ip"
worker_pool_size: 100
tombstone_members_size: 100
Maybe create new section
frostfs
for these parameters?frostfs
section is already in config, I can move these parameters there. Maybe it's worth creating subsection for all tombstone-related parameters infrostfs
section?It's up to you. I'm fine with both options
@ -243,6 +246,8 @@ source_ip_header: "Source-Ip"
| `allowed_access_key_id_prefixes` | `[]string` | no | | List of allowed `AccessKeyID` prefixes which S3 GW serve. If the parameter is omitted, all `AccessKeyID` will be accepted. |
| `reconnect_interval` | `duration` | no | `1m` | Listeners reconnection interval. |
| `source_ip_header` | `string` | yes | | Custom header to retrieve Source IP. |
| `worker_pool_size` | `int` | no | `100` | Maximum worker count in layer's worker pool. |
Probably we should use more specific name for this parameter. I mean mention that this worker pool relates to object removal / putting tombstone
question: What about using generic worker-pools
layer-
orapp-wide
? For example, there is already a worker pool in index pages. Couldn we reuse it? Or it may cause too large job queue?Index pages are only in http-gw, no?
Oh, yes, I got it wrong
@ -216,2 +216,4 @@
obj.SetPayloadSize(prm.PayloadSize)
if prm.Type > 0 {
obj.SetType(prm.Type)
It seems we can unconditionally set type (0 means regular object)
4305c8507f
to037e972424
@ -75,2 +75,4 @@
defaultRetryMaxBackoff = 30 * time.Second
defaultRetryStrategy = handler.RetryStrategyExponential
defaultTombstoneLifetime = uint64(10)
We can write just
@ -166,0 +166,4 @@
# Tombstone's lifetime in epochs.
S3_GW_FROSTFS_TOMBSTONE_LIFETIME=10
# Maximum number of object IDs in one tombstone.
S3_GW_FROSTFS_TOMBSTONE_MEMBERS_COUNT=100
It seems the variable must be
@ -202,0 +203,4 @@
# Tombstone's lifetime in epochs.
lifetime: 10
# Maximum number of object IDs in one tombstone.
members_count: 100
It seems the variable must be
LGTM, but correct (in code or in config examples) config value name
members_count
ormembers_size
037e972424
tod225d24928
New commits pushed, approval review dismissed automatically according to repository settings
Overall LGTM, consider splitting layer chainges into separate file. But this is not required.
@ -264,2 +273,3 @@
func (t *TestFrostFS) CreateObject(_ context.Context, prm frostfs.PrmObjectCreate) (*frostfs.CreateObjectResult, error) {
func (t *TestFrostFS) CreateObject(ctx context.Context, prm frostfs.PrmObjectCreate) (*frostfs.CreateObjectResult, error) {
if prm.Type == object.TypeTombstone {
nitpick: I think this looks better in separate private
This condition does not reuse any code from
CreateObject
so there is no point to inline this code.@ -772,0 +788,4 @@
return nil
}
func (n *Layer) putTombstones(ctx context.Context, bkt *data.BucketInfo, networkInfo netmap.NetworkInfo, members []oid.ID) {
What you think about having all these tombstone collecting function (
putTombstones
,submitPutTombstone
,putTombstoneObject
,prepareTokensParameter
) in separate file, e.g.tombstones.go
? It would be much easier to follow and update this code, on my opinion.@ -214,6 +215,7 @@ func (x *FrostFS) CreateObject(ctx context.Context, prm frostfs.PrmObjectCreate)
obj.SetOwnerID(x.owner)
obj.SetAttributes(attrs...)
obj.SetPayloadSize(prm.PayloadSize)
obj.SetType(prm.Type)
Is default type a regular object? As far as I understand,
prm.Type
is optional.Yes, default type is regular
d225d24928
to309b53f1fb
New commits pushed, approval review dismissed automatically according to repository settings
New commits pushed, approval review dismissed automatically according to repository settings
New commits pushed, approval review dismissed automatically according to repository settings
309b53f1fb
tof215d200e8