From f4fef9c02eb9f333ac0e0bd56201149ea158157c Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 27 Jan 2025 14:17:58 +0300 Subject: [PATCH 1/5] [#1616] getsvc: Move head assembling to a separate file Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/assembler.go | 37 ------------------- pkg/services/object/get/assembler_head.go | 45 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 37 deletions(-) create mode 100644 pkg/services/object/get/assembler_head.go diff --git a/pkg/services/object/get/assembler.go b/pkg/services/object/get/assembler.go index ff3f90bf2..23fc187f5 100644 --- a/pkg/services/object/get/assembler.go +++ b/pkg/services/object/get/assembler.go @@ -71,43 +71,6 @@ func (a *assembler) Assemble(ctx context.Context, writer ObjectWriter) (*objectS return a.parentObject, nil } -func (a *assembler) assembleHeader(ctx context.Context, writer ObjectWriter) (*objectSDK.Object, error) { - var sourceObjectIDs []oid.ID - sourceObjectID, ok := a.splitInfo.Link() - if ok { - sourceObjectIDs = append(sourceObjectIDs, sourceObjectID) - } - sourceObjectID, ok = a.splitInfo.LastPart() - if ok { - sourceObjectIDs = append(sourceObjectIDs, sourceObjectID) - } - if len(sourceObjectIDs) == 0 { - return nil, objectSDK.NewSplitInfoError(a.splitInfo) - } - for _, sourceObjectID = range sourceObjectIDs { - obj, err := a.getParent(ctx, sourceObjectID, writer) - if err == nil { - return obj, nil - } - } - return nil, objectSDK.NewSplitInfoError(a.splitInfo) -} - -func (a *assembler) getParent(ctx context.Context, sourceObjectID oid.ID, writer ObjectWriter) (*objectSDK.Object, error) { - obj, err := a.objGetter.HeadObject(ctx, sourceObjectID) - if err != nil { - return nil, err - } - parent := obj.Parent() - if parent == nil { - return nil, objectSDK.NewSplitInfoError(a.splitInfo) - } - if err := writer.WriteHeader(ctx, parent); err != nil { - return nil, err - } - return obj, nil -} - func (a *assembler) getLastPartOrLinkObjectID() (oid.ID, bool) { sourceObjectID, ok := a.splitInfo.Link() if ok { diff --git a/pkg/services/object/get/assembler_head.go b/pkg/services/object/get/assembler_head.go new file mode 100644 index 000000000..ff213cb82 --- /dev/null +++ b/pkg/services/object/get/assembler_head.go @@ -0,0 +1,45 @@ +package getsvc + +import ( + "context" + + objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" + oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" +) + +func (a *assembler) assembleHeader(ctx context.Context, writer ObjectWriter) (*objectSDK.Object, error) { + var sourceObjectIDs []oid.ID + sourceObjectID, ok := a.splitInfo.Link() + if ok { + sourceObjectIDs = append(sourceObjectIDs, sourceObjectID) + } + sourceObjectID, ok = a.splitInfo.LastPart() + if ok { + sourceObjectIDs = append(sourceObjectIDs, sourceObjectID) + } + if len(sourceObjectIDs) == 0 { + return nil, objectSDK.NewSplitInfoError(a.splitInfo) + } + for _, sourceObjectID = range sourceObjectIDs { + obj, err := a.getParent(ctx, sourceObjectID, writer) + if err == nil { + return obj, nil + } + } + return nil, objectSDK.NewSplitInfoError(a.splitInfo) +} + +func (a *assembler) getParent(ctx context.Context, sourceObjectID oid.ID, writer ObjectWriter) (*objectSDK.Object, error) { + obj, err := a.objGetter.HeadObject(ctx, sourceObjectID) + if err != nil { + return nil, err + } + parent := obj.Parent() + if parent == nil { + return nil, objectSDK.NewSplitInfoError(a.splitInfo) + } + if err := writer.WriteHeader(ctx, parent); err != nil { + return nil, err + } + return obj, nil +} -- 2.45.3 From c7b406041fbe8a464d6dc98a7f62fe1c0e6b6cb5 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 27 Jan 2025 14:22:46 +0300 Subject: [PATCH 2/5] [#1616] getsvc: Move range assembling to a separate file Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/assembler.go | 102 ++++++-------------- pkg/services/object/get/assembler_range.go | 103 +++++++++++++++++++++ 2 files changed, 129 insertions(+), 76 deletions(-) create mode 100644 pkg/services/object/get/assembler_range.go diff --git a/pkg/services/object/get/assembler.go b/pkg/services/object/get/assembler.go index 23fc187f5..886f0aabb 100644 --- a/pkg/services/object/get/assembler.go +++ b/pkg/services/object/get/assembler.go @@ -59,15 +59,23 @@ func (a *assembler) Assemble(ctx context.Context, writer ObjectWriter) (*objectS if previousID == nil && len(childrenIDs) == 0 { return nil, objectSDK.NewSplitInfoError(a.splitInfo) } + if len(childrenIDs) > 0 { - if err := a.assembleObjectByChildrenList(ctx, childrenIDs, writer); err != nil { - return nil, err + if a.rng != nil { + err = a.assembleObjectByChildrenListRange(ctx, childrenIDs, writer) + } else { + err = a.assembleObjectByChildrenList(ctx, childrenIDs, writer) } } else { - if err := a.assemleObjectByPreviousIDInReverse(ctx, *previousID, writer); err != nil { - return nil, err + if a.rng != nil { + err = a.assemleObjectByPreviousIDInReverseRange(ctx, *previousID, writer) + } else { + err = a.assemleObjectByPreviousIDInReverse(ctx, *previousID, writer) } } + if err != nil { + return nil, err + } return a.parentObject, nil } @@ -153,26 +161,16 @@ func (a *assembler) getChildObject(ctx context.Context, id oid.ID, rng *objectSD } func (a *assembler) assembleObjectByChildrenList(ctx context.Context, childrenIDs []oid.ID, writer ObjectWriter) error { - if a.rng == nil { - if err := writer.WriteHeader(ctx, a.parentObject.CutPayload()); err != nil { - return err - } - return a.assemblePayloadByObjectIDs(ctx, writer, childrenIDs, nil, true) - } - - if err := a.assemblePayloadInReverse(ctx, writer, childrenIDs[len(childrenIDs)-1]); err != nil { + if err := writer.WriteHeader(ctx, a.parentObject.CutPayload()); err != nil { return err } - return writer.WriteChunk(ctx, a.parentObject.Payload()) + return a.assemblePayloadByObjectIDs(ctx, writer, childrenIDs, true) } func (a *assembler) assemleObjectByPreviousIDInReverse(ctx context.Context, prevID oid.ID, writer ObjectWriter) error { - if a.rng == nil { - if err := writer.WriteHeader(ctx, a.parentObject.CutPayload()); err != nil { - return err - } + if err := writer.WriteHeader(ctx, a.parentObject.CutPayload()); err != nil { + return err } - if err := a.assemblePayloadInReverse(ctx, writer, prevID); err != nil { return err } @@ -182,16 +180,9 @@ func (a *assembler) assemleObjectByPreviousIDInReverse(ctx context.Context, prev return nil } -func (a *assembler) assemblePayloadByObjectIDs(ctx context.Context, writer ObjectWriter, partIDs []oid.ID, partRanges []objectSDK.Range, verifyIsChild bool) error { - withRng := len(partRanges) > 0 && a.rng != nil - +func (a *assembler) assemblePayloadByObjectIDs(ctx context.Context, writer ObjectWriter, partIDs []oid.ID, verifyIsChild bool) error { for i := range partIDs { - var r *objectSDK.Range - if withRng { - r = &partRanges[i] - } - - _, err := a.getChildObject(ctx, partIDs[i], r, verifyIsChild, writer) + _, err := a.getChildObject(ctx, partIDs[i], nil, verifyIsChild, writer) if err != nil { return err } @@ -200,22 +191,16 @@ func (a *assembler) assemblePayloadByObjectIDs(ctx context.Context, writer Objec } func (a *assembler) assemblePayloadInReverse(ctx context.Context, writer ObjectWriter, prevID oid.ID) error { - chain, rngs, err := a.buildChain(ctx, prevID) + chain, err := a.buildChain(ctx, prevID) if err != nil { return err } - reverseRngs := len(rngs) > 0 - for left, right := 0, len(chain)-1; left < right; left, right = left+1, right-1 { chain[left], chain[right] = chain[right], chain[left] - - if reverseRngs { - rngs[left], rngs[right] = rngs[right], rngs[left] - } } - return a.assemblePayloadByObjectIDs(ctx, writer, chain, rngs, false) + return a.assemblePayloadByObjectIDs(ctx, writer, chain, false) } func (a *assembler) isChild(obj *objectSDK.Object) bool { @@ -223,63 +208,28 @@ func (a *assembler) isChild(obj *objectSDK.Object) bool { return parent == nil || equalAddresses(a.addr, object.AddressOf(parent)) } -func (a *assembler) buildChain(ctx context.Context, prevID oid.ID) ([]oid.ID, []objectSDK.Range, error) { +func (a *assembler) buildChain(ctx context.Context, prevID oid.ID) ([]oid.ID, error) { var ( chain []oid.ID - rngs []objectSDK.Range - from = a.rng.GetOffset() - to = from + a.rng.GetLength() hasPrev = true ) // fill the chain end-to-start for hasPrev { - // check that only for "range" requests, - // for `GET` it stops via the false `withPrev` - if a.rng != nil && a.currentOffset <= from { - break - } - head, err := a.objGetter.HeadObject(ctx, prevID) if err != nil { - return nil, nil, err + return nil, err } if !a.isChild(head) { - return nil, nil, errParentAddressDiffers + return nil, errParentAddressDiffers } - if a.rng != nil { - sz := head.PayloadSize() - - a.currentOffset -= sz - - if a.currentOffset < to { - off := uint64(0) - if from > a.currentOffset { - off = from - a.currentOffset - sz -= from - a.currentOffset - } - - if to < a.currentOffset+off+sz { - sz = to - off - a.currentOffset - } - - index := len(rngs) - rngs = append(rngs, objectSDK.Range{}) - rngs[index].SetOffset(off) - rngs[index].SetLength(sz) - - id, _ := head.ID() - chain = append(chain, id) - } - } else { - id, _ := head.ID() - chain = append(chain, id) - } + id, _ := head.ID() + chain = append(chain, id) prevID, hasPrev = head.PreviousID() } - return chain, rngs, nil + return chain, nil } diff --git a/pkg/services/object/get/assembler_range.go b/pkg/services/object/get/assembler_range.go new file mode 100644 index 000000000..638db0c76 --- /dev/null +++ b/pkg/services/object/get/assembler_range.go @@ -0,0 +1,103 @@ +package getsvc + +import ( + "context" + + objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" + oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" +) + +func (a *assembler) assembleObjectByChildrenListRange(ctx context.Context, childrenIDs []oid.ID, writer ObjectWriter) error { + if err := a.assemblePayloadInReverseRange(ctx, writer, childrenIDs[len(childrenIDs)-1]); err != nil { + return err + } + return writer.WriteChunk(ctx, a.parentObject.Payload()) +} + +func (a *assembler) assemleObjectByPreviousIDInReverseRange(ctx context.Context, prevID oid.ID, writer ObjectWriter) error { + if err := a.assemblePayloadInReverseRange(ctx, writer, prevID); err != nil { + return err + } + if err := writer.WriteChunk(ctx, a.parentObject.Payload()); err != nil { // last part + return err + } + return nil +} + +func (a *assembler) assemblePayloadByObjectIDsRange(ctx context.Context, writer ObjectWriter, partIDs []oid.ID, partRanges []objectSDK.Range) error { + for i := range partIDs { + _, err := a.getChildObject(ctx, partIDs[i], &partRanges[i], false, writer) + if err != nil { + return err + } + } + return nil +} + +func (a *assembler) assemblePayloadInReverseRange(ctx context.Context, writer ObjectWriter, prevID oid.ID) error { + chain, rngs, err := a.buildChainRange(ctx, prevID) + if err != nil { + return err + } + + for left, right := 0, len(chain)-1; left < right; left, right = left+1, right-1 { + chain[left], chain[right] = chain[right], chain[left] + rngs[left], rngs[right] = rngs[right], rngs[left] + } + + return a.assemblePayloadByObjectIDsRange(ctx, writer, chain, rngs) +} + +func (a *assembler) buildChainRange(ctx context.Context, prevID oid.ID) ([]oid.ID, []objectSDK.Range, error) { + var ( + chain []oid.ID + rngs []objectSDK.Range + from = a.rng.GetOffset() + to = from + a.rng.GetLength() + + hasPrev = true + ) + + // fill the chain end-to-start + for hasPrev { + if a.currentOffset <= from { + break + } + + head, err := a.objGetter.HeadObject(ctx, prevID) + if err != nil { + return nil, nil, err + } + if !a.isChild(head) { + return nil, nil, errParentAddressDiffers + } + + sz := head.PayloadSize() + + a.currentOffset -= sz + + if a.currentOffset < to { + off := uint64(0) + if from > a.currentOffset { + off = from - a.currentOffset + sz -= from - a.currentOffset + } + + if to < a.currentOffset+off+sz { + sz = to - off - a.currentOffset + } + + index := len(rngs) + rngs = append(rngs, objectSDK.Range{}) + rngs[index].SetOffset(off) + rngs[index].SetLength(sz) + + id, _ := head.ID() + chain = append(chain, id) + } + + prevID, hasPrev = head.PreviousID() + } + + return chain, rngs, nil +} -- 2.45.3 From 7d1ff316e02c1bc8ec61d36b596ef9178356b496 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 27 Jan 2025 14:24:06 +0300 Subject: [PATCH 3/5] [#1616] getsvc: Use slices.Reverse() where possible Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/assembler.go | 6 ++---- pkg/services/object/get/assembler_range.go | 8 +++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/pkg/services/object/get/assembler.go b/pkg/services/object/get/assembler.go index 886f0aabb..b24c9417b 100644 --- a/pkg/services/object/get/assembler.go +++ b/pkg/services/object/get/assembler.go @@ -2,6 +2,7 @@ package getsvc import ( "context" + "slices" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" @@ -196,10 +197,7 @@ func (a *assembler) assemblePayloadInReverse(ctx context.Context, writer ObjectW return err } - for left, right := 0, len(chain)-1; left < right; left, right = left+1, right-1 { - chain[left], chain[right] = chain[right], chain[left] - } - + slices.Reverse(chain) return a.assemblePayloadByObjectIDs(ctx, writer, chain, false) } diff --git a/pkg/services/object/get/assembler_range.go b/pkg/services/object/get/assembler_range.go index 638db0c76..748a499ef 100644 --- a/pkg/services/object/get/assembler_range.go +++ b/pkg/services/object/get/assembler_range.go @@ -2,6 +2,7 @@ package getsvc import ( "context" + "slices" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" @@ -40,11 +41,8 @@ func (a *assembler) assemblePayloadInReverseRange(ctx context.Context, writer Ob return err } - for left, right := 0, len(chain)-1; left < right; left, right = left+1, right-1 { - chain[left], chain[right] = chain[right], chain[left] - rngs[left], rngs[right] = rngs[right], rngs[left] - } - + slices.Reverse(chain) + slices.Reverse(rngs) return a.assemblePayloadByObjectIDsRange(ctx, writer, chain, rngs) } -- 2.45.3 From bf80e9c7e00cea47af40c2b8bcfde36deaedea37 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Tue, 28 Jan 2025 15:01:02 +0300 Subject: [PATCH 4/5] [#1616] getsvc: Simplify buildChainRange() Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/assembler_range.go | 24 +++++++--------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/pkg/services/object/get/assembler_range.go b/pkg/services/object/get/assembler_range.go index 748a499ef..844a471a6 100644 --- a/pkg/services/object/get/assembler_range.go +++ b/pkg/services/object/get/assembler_range.go @@ -70,30 +70,20 @@ func (a *assembler) buildChainRange(ctx context.Context, prevID oid.ID) ([]oid.I return nil, nil, errParentAddressDiffers } - sz := head.PayloadSize() - - a.currentOffset -= sz - - if a.currentOffset < to { - off := uint64(0) - if from > a.currentOffset { - off = from - a.currentOffset - sz -= from - a.currentOffset - } - - if to < a.currentOffset+off+sz { - sz = to - off - a.currentOffset - } - + nextOffset := a.currentOffset - head.PayloadSize() + clampedFrom := max(from, nextOffset) + clampedTo := min(to, a.currentOffset) + if clampedFrom < clampedTo { index := len(rngs) rngs = append(rngs, objectSDK.Range{}) - rngs[index].SetOffset(off) - rngs[index].SetLength(sz) + rngs[index].SetOffset(clampedFrom - nextOffset) + rngs[index].SetLength(clampedTo - clampedFrom) id, _ := head.ID() chain = append(chain, id) } + a.currentOffset = nextOffset prevID, hasPrev = head.PreviousID() } -- 2.45.3 From 8d9a578db7b7cecb9db48abd36ee54a7779667a3 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 29 Jan 2025 12:23:05 +0300 Subject: [PATCH 5/5] [#1616] getsvc: Move break condition from body to the loop condition Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/assembler_range.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/services/object/get/assembler_range.go b/pkg/services/object/get/assembler_range.go index 844a471a6..780693c40 100644 --- a/pkg/services/object/get/assembler_range.go +++ b/pkg/services/object/get/assembler_range.go @@ -57,11 +57,7 @@ func (a *assembler) buildChainRange(ctx context.Context, prevID oid.ID) ([]oid.I ) // fill the chain end-to-start - for hasPrev { - if a.currentOffset <= from { - break - } - + for hasPrev && from < a.currentOffset { head, err := a.objGetter.HeadObject(ctx, prevID) if err != nil { return nil, nil, err -- 2.45.3