TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 105 Pull requests 10 Projects Releases 2 Activity Actions

Fix PutSingle with OID that was already removed #1579

Merged
fyrchik merged 2 commits from aarifullin/frostfs-node:fix/put_single into master 2024-12-26 11:27:56 +00:00
Conversation 0 Commits 2 Files changed 2 +4
2 changed files with 4 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit a0c261104e - Show all commits

3
pkg/services/object/common/writer/ec.go
View file

@ -14,6 +14,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/policy"
svcutil "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object_manager/placement"
clientSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client"
containerSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container"
objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/erasurecode"
@ -274,6 +275,8 @@ func (e *ECWriter) writePart(ctx context.Context, obj *objectSDK.Object, partIdx
err := e.putECPartToNode(ctx, obj, node)
if err == nil {
return nil
} else if clientSDK.IsErrObjectAlreadyRemoved(err) {
return err
}
e.Config.Logger.Warn(ctx, logs.ECFailedToSaveECPart, zap.Stringer("part_address", object.AddressOf(obj)),
zap.Stringer("parent_address", obj.ECHeader().Parent()), zap.Int("part_index", partIdx),

1
pkg/services/object/put/single.go
View file

@ -352,6 +352,7 @@ func (s *Service) redirectPutSingleRequest(ctx context.Context,
err = signature.VerifyServiceMessage(resp)
if err != nil {
err = fmt.Errorf("response verification failed: %w", err)
return
}
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-12-23 12:12:59 +00:00
Outdated
Review
Copy link

Could you point me at an example of where we already use similar logic?
These lines seem out of place to me.

Could you point me at an example of where we already use similar logic? These lines seem out of place to me.
aarifullin commented 2024-12-23 13:04:28 +00:00
Outdated
Review
Copy link

https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/client/object_put_single.go#L162-L174
https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/client/common.go#L100

https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/client/object_put_single.go#L162-L174 https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/client/common.go#L100
fyrchik commented 2024-12-24 07:16:02 +00:00
Outdated
Review
Copy link

Both links are from the SDK, I mean in the node.
This line is rather generic, I do not understand why PutSingle is special.

Both links are from the SDK, I mean in the node. This line is rather generic, I do not understand why `PutSingle` is special.
aarifullin commented 2024-12-24 07:43:11 +00:00
Outdated
Review
Copy link

headRequest -> verifyResponse

[headRequest](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/get/v2/head_forwarder.go#L57) -> [verifyResponse](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/services/object/get/v2/util.go#L419-L429)
st := apistatus.FromStatusV2(resp.GetMetaHeader().GetStatus())
dstepanov-yadro commented 2024-12-25 06:17:52 +00:00
Review
Copy link
		err = signature.VerifyServiceMessage(resp)
		if err != nil {
			err = fmt.Errorf("response verification failed: %w", err)
			return     <----------------------- add this
		}

		st := apistatus.FromStatusV2(resp.GetMetaHeader().GetStatus())
		err = apistatus.ErrFromStatus(st)
``` err = signature.VerifyServiceMessage(resp) if err != nil { err = fmt.Errorf("response verification failed: %w", err) return <----------------------- add this } st := apistatus.FromStatusV2(resp.GetMetaHeader().GetStatus()) err = apistatus.ErrFromStatus(st) ```
aarifullin commented 2024-12-25 08:23:54 +00:00
Review
Copy link

Correct! Fixed

Correct! Fixed