From 492d4fc269dfdc5fab6638d968964b41dfa9595b Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Sat, 12 Sep 2026 09:22:32 +0300 Subject: [PATCH 1/6] sn/object: Do not log result of GET with TTL=1 twice Was caused by a method call without changing the state. Before: ``` 2026-09-12T09:24:20.703+0300 debug get/get.go:239 operation finished with error {"component": "Object.Get service", "address": "3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2/3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2", "raw": false, "local": true, "with session": false, "with bearer": false, "request": "GET", "error": "status: code = 2049 message = object not found"} 2026-09-12T09:24:20.703+0300 debug get/container.go:11 return result directly {"component": "Object.Get service", "address": "3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2/3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2", "raw": false, "local": true, "with session": false, "with bearer": false, "request": "GET"} 2026-09-12T09:24:20.703+0300 debug get/get.go:239 operation finished with error {"component": "Object.Get service", "address": "3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2/3VkN6ekDbYnYrDAgmsj5c66GvDLaRVFZj4SBZhj6R8G2", "raw": false, "local": true, "with session": false, "with bearer": false, "request": "GET", "error": "status: code = 2049 message = object not found"} ``` Now there will be no repeat message. Signed-off-by: Leonard Liubich --- pkg/services/object/get/container.go | 5 ----- pkg/services/object/get/get.go | 8 ++++++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/services/object/get/container.go b/pkg/services/object/get/container.go index 6617505089..acf0495f56 100644 --- a/pkg/services/object/get/container.go +++ b/pkg/services/object/get/container.go @@ -7,11 +7,6 @@ import ( ) func (exec *execCtx) executeOnContainer() { - if exec.isLocal() { - exec.log.Debug("return result directly") - return - } - exec.log.Debug("trying to execute in container...") addr := exec.address() diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index 89120408f0..260fd6f063 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -223,6 +223,10 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { } exec.assemble() if errors.Is(exec.err, errNoLinkNoLastPart) && execCnr { + if exec.isLocal() { + exec.log.Debug("return result directly") + return + } exec.executeOnContainer() exec.analyzeStatus(false) } @@ -235,6 +239,10 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { ) if execCnr && !exec.responseStarted { + if exec.isLocal() { + exec.log.Debug("return result directly") + return + } exec.executeOnContainer() exec.analyzeStatus(false) } From cb4895243dfafe1abec17c04c3286cb913d32d91 Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Sat, 12 Sep 2026 09:29:30 +0300 Subject: [PATCH 2/6] sn/object: Deduplicate multiline code block in GET handler Refactoring purpose. Signed-off-by: Leonard Liubich --- pkg/services/object/get/get.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index 260fd6f063..b77f4142f4 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -216,19 +216,15 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { switch exec.status { case statusOK: exec.log.Debug("operation finished successfully") + return case statusVIRTUAL: exec.log.Debug("requested object is virtual") if exec.collectOnly { return } exec.assemble() - if errors.Is(exec.err, errNoLinkNoLastPart) && execCnr { - if exec.isLocal() { - exec.log.Debug("return result directly") - return - } - exec.executeOnContainer() - exec.analyzeStatus(false) + if !errors.Is(exec.err, errNoLinkNoLastPart) || !execCnr { + return } case statusAPIResponse: exec.log.Debug("received api response locally, return directly", zap.Error(exec.err)) @@ -238,13 +234,15 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { zap.Error(exec.err), ) - if execCnr && !exec.responseStarted { - if exec.isLocal() { - exec.log.Debug("return result directly") - return - } - exec.executeOnContainer() - exec.analyzeStatus(false) + if !execCnr || exec.responseStarted { + return } } + + if exec.isLocal() { + exec.log.Debug("return result directly") + return + } + exec.executeOnContainer() + exec.analyzeStatus(false) } From e1e1d5be215b45b283d3bef2f351bb5db3c9c35d Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Sun, 13 Sep 2026 05:53:58 +0300 Subject: [PATCH 3/6] sn/object: Refactor recursive method in GET handler The final goal is to not make local operation blindly but when handling sorted container node list. Also, a bit more code is usually more understandable than recursion. Signed-off-by: Leonard Liubich --- pkg/services/object/get/get.go | 72 ++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index b77f4142f4..115f59fe38 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -207,42 +207,66 @@ func (exec *execCtx) execute() { // perform local operation exec.executeLocal() + if exec.analyzeLocalStatus() { + return + } + + if exec.isLocal() { + exec.log.Debug("return result directly") + return + } - exec.analyzeStatus(true) + exec.executeOnContainer() + exec.analyzeContainerStatus() } -func (exec *execCtx) analyzeStatus(execCnr bool) { +func (exec *execCtx) analyzeLocalStatus() bool { // analyze local result switch exec.status { case statusOK: - exec.log.Debug("operation finished successfully") - return + exec.logSuccess() case statusVIRTUAL: - exec.log.Debug("requested object is virtual") - if exec.collectOnly { - return - } - exec.assemble() - if !errors.Is(exec.err, errNoLinkNoLastPart) || !execCnr { - return - } + exec.handleVirtualObject() + return !errors.Is(exec.err, errNoLinkNoLastPart) case statusAPIResponse: - exec.log.Debug("received api response locally, return directly", zap.Error(exec.err)) - return + exec.logAPIError() default: - exec.log.Debug("operation finished with error", - zap.Error(exec.err), - ) + exec.logOtherError() + return exec.responseStarted + } - if !execCnr || exec.responseStarted { - return - } + return true +} + +func (exec *execCtx) analyzeContainerStatus() { + switch exec.status { + case statusOK: + exec.logSuccess() + case statusVIRTUAL: + exec.handleVirtualObject() + case statusAPIResponse: + exec.logAPIError() + default: + exec.logOtherError() } +} - if exec.isLocal() { - exec.log.Debug("return result directly") +func (exec *execCtx) handleVirtualObject() { + exec.log.Debug("requested object is virtual") + if exec.collectOnly { return } - exec.executeOnContainer() - exec.analyzeStatus(false) + exec.assemble() +} + +func (exec *execCtx) logSuccess() { + exec.log.Debug("operation finished successfully") +} + +func (exec *execCtx) logAPIError() { + exec.log.Debug("received api response locally, return directly", zap.Error(exec.err)) +} + +func (exec *execCtx) logOtherError() { + exec.log.Debug("operation finished with error", zap.Error(exec.err)) } From bbe64dba5592b413ab364bbd857145313f3f5ade Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Sun, 13 Sep 2026 06:24:06 +0300 Subject: [PATCH 4/6] sn/object: Do not access local storage to find a replica blindly Previously, GET/HEAD server was always looked up for the object locally first, regardless of the placement policy. This introduced overhead in the case of optimal placement. The main motivation was to maintain object availability when its replica was moved to a reserve container node or if it became stuck outside the container. Since this is also a live case in a storage distributed system, this preserves local reading but as a fallback. In the context of performance on reading with a uniform load distribution across the entire cluster, an increase in RPS is not expected since the costs are negligible relative to actual DB/FS reading and RPC. However, with optimal placement (which is the target), the processing speed of a single request is guaranteed to increase, and the cost of a DB transaction (in particular, allocs) will disappear. Closes #3411. Signed-off-by: Leonard Liubich --- CHANGELOG.md | 1 + internal/netmap/netmap.go | 6 ++-- pkg/services/object/get/container.go | 47 ++++++++++++++++++++++++++-- pkg/services/object/get/get.go | 9 ++---- pkg/services/object/get/get_test.go | 13 +++++--- 5 files changed, 60 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a458c0ee1b..5b05c92ea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Changelog for NeoFS Node - Policer now uses an in-memory queue for redundant object removal, reducing metabase writes (#4216) - Session token storage now indexes tokens by expiration epoch for efficient cleanup (#4243) - Session key encryption now uses HPKE (#4247) +- SN object GET/HEAD server now traverses nodes strictly by weight, and falls back to local storage (#4232) ### Removed - fstree-decompress utility (#4223) diff --git a/internal/netmap/netmap.go b/internal/netmap/netmap.go index 61e3def1cd..8f7059148e 100644 --- a/internal/netmap/netmap.go +++ b/internal/netmap/netmap.go @@ -11,11 +11,13 @@ import ( // least one element from nodeSets. func NodeSetsContainPublicKeyFunc(nodeSets [][]netmap.NodeInfo, checkPubKeyFn func([]byte) bool) bool { return slices.ContainsFunc(nodeSets, func(nodeSet []netmap.NodeInfo) bool { - return nodeSetContainsPublicKeyFunc(nodeSet, checkPubKeyFn) + return NodeSetContainsPublicKeyFunc(nodeSet, checkPubKeyFn) }) } -func nodeSetContainsPublicKeyFunc(nodeSet []netmap.NodeInfo, checkPubKeyFn func([]byte) bool) bool { +// NodeSetContainsPublicKeyFunc checks whether checkPubKeyFn is true for at +// least one element from nodeSet. +func NodeSetContainsPublicKeyFunc(nodeSet []netmap.NodeInfo, checkPubKeyFn func([]byte) bool) bool { return slices.ContainsFunc(nodeSet, func(node netmap.NodeInfo) bool { return checkPubKeyFn(node.PublicKey()) }) diff --git a/pkg/services/object/get/container.go b/pkg/services/object/get/container.go index acf0495f56..52cc50c178 100644 --- a/pkg/services/object/get/container.go +++ b/pkg/services/object/get/container.go @@ -2,7 +2,9 @@ package getsvc import ( "context" + "errors" + inetmap "github.com/nspcc-dev/neofs-node/internal/netmap" "go.uber.org/zap" ) @@ -20,10 +22,30 @@ func (exec *execCtx) executeOnContainer() { exec.status = statusUndefined exec.err = err exec.log.Debug("failed to list storage nodes for the object", zap.Error(err)) + exec.analyzeContainerStatus() return } } + var localDone bool + execLocal := func() bool { + if localDone { + return false + } + localDone = true + exec.executeLocal() + return exec.analyzeLocalStatus() + } + + // try local node first if it is a primary holder + for i, n := range primaryCounts { + if inetmap.NodeSetContainsPublicKeyFunc(nodeLists[i][:n], exec.svc.neoFSNet.IsLocalNodePublicKey) { + if execLocal() { + return + } + } + } + ctx, cancel := context.WithCancel(exec.context()) defer cancel() @@ -45,14 +67,24 @@ func (exec *execCtx) executeOnContainer() { exec.log.Debug("interrupt placement iteration by context", zap.Error(ctx.Err()), ) - + if exec.err == nil { + exec.err = ctx.Err() + } + exec.analyzeContainerStatus() return default: } bKey := nodeLists[i][j].PublicKey() strKey := string(bKey) - if _, ok := mProcessedNodes[strKey]; ok || exec.svc.neoFSNet.IsLocalNodePublicKey(bKey) { + if _, ok := mProcessedNodes[strKey]; ok { + continue + } + + if exec.svc.neoFSNet.IsLocalNodePublicKey(bKey) { + if execLocal() { + return + } continue } @@ -63,7 +95,10 @@ func (exec *execCtx) executeOnContainer() { // we reach the best result - split info with linking object ID. if exec.processNode(nodeLists[i][j]) { exec.log.Debug("completing the operation") - return + exec.analyzeContainerStatus() + if exec.responseStarted || !errors.Is(exec.err, errNoLinkNoLastPart) { + return + } } } @@ -73,4 +108,10 @@ func (exec *execCtx) executeOnContainer() { i = -1 } } + + // Local node is not in the container now. But it can still store the object. + // One scenario is when the container nodes have changed, the local node is no + // longer in it, and the object hasn't migrated yet. To cover such cases, lookup + // local storage as a last-ditch effort to find the object. + execLocal() } diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index 115f59fe38..8cdbf94bd1 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -205,19 +205,14 @@ func (s *Service) get(ctx context.Context, prm commonPrm, opts ...execOption) st func (exec *execCtx) execute() { exec.log.Debug("serving request...") - // perform local operation - exec.executeLocal() - if exec.analyzeLocalStatus() { - return - } - if exec.isLocal() { + exec.executeLocal() + exec.analyzeLocalStatus() exec.log.Debug("return result directly") return } exec.executeOnContainer() - exec.analyzeContainerStatus() } func (exec *execCtx) analyzeLocalStatus() bool { diff --git a/pkg/services/object/get/get_test.go b/pkg/services/object/get/get_test.go index f61eab9973..a6d57337da 100644 --- a/pkg/services/object/get/get_test.go +++ b/pkg/services/object/get/get_test.go @@ -92,7 +92,8 @@ func TestGetExtendedRangeOfECPart(t *testing.T) { } type testNeoFS struct { - vectors map[oid.Address][][]netmap.NodeInfo + vectors map[oid.Address][][]netmap.NodeInfo + localPublicKey []byte } type testClientCache struct { @@ -116,7 +117,9 @@ func newTestStorage() *testStorage { } } -func (g *testNeoFS) IsLocalNodePublicKey([]byte) bool { return false } +func (g *testNeoFS) IsLocalNodePublicKey(pubKey []byte) bool { + return bytes.Equal(g.localPublicKey, pubKey) +} func (g *testNeoFS) GetNodesForObject(addr oid.Address) ([][]netmap.NodeInfo, []uint, []iec.Rule, error) { nodeLists, ok := g.vectors[addr] @@ -1404,7 +1407,7 @@ func TestDoubleWriteHeaderOnPayloadReadFailure(t *testing.T) { err: readErr, } - anyNodeLists, nodeStrs := testNodeMatrix(t, []int{1}) + anyNodeLists, nodeStrs := testNodeMatrix(t, []int{2}) clientCache := &testClientCache{ clients: make(map[string]*testClient), @@ -1422,6 +1425,7 @@ func TestDoubleWriteHeaderOnPayloadReadFailure(t *testing.T) { vectors: map[oid.Address][][]netmap.NodeInfo{ addr: anyNodeLists, }, + localPublicKey: anyNodeLists[0][0].PublicKey(), } writer := &trackingWriter{} @@ -1452,7 +1456,7 @@ func TestDoubleWriteHeaderOnChunkWriteFailure(t *testing.T) { storage := newTestStorage() storage.addPhy(addr, obj) - anyNodeLists, nodeStrs := testNodeMatrix(t, []int{1}) + anyNodeLists, nodeStrs := testNodeMatrix(t, []int{2}) clientCache := &testClientCache{ clients: make(map[string]*testClient), @@ -1470,6 +1474,7 @@ func TestDoubleWriteHeaderOnChunkWriteFailure(t *testing.T) { vectors: map[oid.Address][][]netmap.NodeInfo{ addr: anyNodeLists, }, + localPublicKey: anyNodeLists[0][0].PublicKey(), } writeErr := errors.New("simulated chunk write error") From 434d44949a888f5c112e33682a131f897bc9bc14 Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Sun, 13 Sep 2026 06:25:16 +0300 Subject: [PATCH 5/6] sn/object: Do not switch over known value in GET handler Signed-off-by: Leonard Liubich --- pkg/services/object/get/container.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/object/get/container.go b/pkg/services/object/get/container.go index 52cc50c178..bc13bb42c9 100644 --- a/pkg/services/object/get/container.go +++ b/pkg/services/object/get/container.go @@ -22,7 +22,7 @@ func (exec *execCtx) executeOnContainer() { exec.status = statusUndefined exec.err = err exec.log.Debug("failed to list storage nodes for the object", zap.Error(err)) - exec.analyzeContainerStatus() + exec.logOtherError() return } } @@ -70,7 +70,7 @@ func (exec *execCtx) executeOnContainer() { if exec.err == nil { exec.err = ctx.Err() } - exec.analyzeContainerStatus() + exec.logOtherError() return default: } From e8346d857c8094b0b5246f35e3eb3a54bff33fee Mon Sep 17 00:00:00 2001 From: Leonard Liubich Date: Wed, 21 Oct 2026 19:49:17 +0300 Subject: [PATCH 6/6] sn/object: Merge two GET handler methods into one They are always called in pairs. Signed-off-by: Leonard Liubich --- pkg/services/object/get/container.go | 3 +-- pkg/services/object/get/get.go | 1 - pkg/services/object/get/local.go | 6 ++++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/services/object/get/container.go b/pkg/services/object/get/container.go index bc13bb42c9..7ffcc1dbac 100644 --- a/pkg/services/object/get/container.go +++ b/pkg/services/object/get/container.go @@ -33,8 +33,7 @@ func (exec *execCtx) executeOnContainer() { return false } localDone = true - exec.executeLocal() - return exec.analyzeLocalStatus() + return exec.executeLocal() } // try local node first if it is a primary holder diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index 8cdbf94bd1..2dca37dc77 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -207,7 +207,6 @@ func (exec *execCtx) execute() { if exec.isLocal() { exec.executeLocal() - exec.analyzeLocalStatus() exec.log.Debug("return result directly") return } diff --git a/pkg/services/object/get/local.go b/pkg/services/object/get/local.go index 438d11e36d..617513c381 100644 --- a/pkg/services/object/get/local.go +++ b/pkg/services/object/get/local.go @@ -13,7 +13,7 @@ import ( "go.uber.org/zap" ) -func (exec *execCtx) executeLocal() { +func (exec *execCtx) executeLocal() bool { var err error exec.collectedHeader, exec.collectedReader, err = exec.svc.localStorage.get(exec) @@ -40,7 +40,7 @@ func (exec *execCtx) executeLocal() { exec.status = statusNotFound exec.err = err - return + break } exec.status = statusAPIResponse @@ -50,6 +50,8 @@ func (exec *execCtx) executeLocal() { mergeSplitInfo(exec.splitInfo(), errSplitInfo.SplitInfo()) exec.err = object.NewSplitInfoError(exec.infoSplit) } + + return exec.analyzeLocalStatus() } func (s *Service) copyLocalObjectHeader(ctx context.Context, dst internal.HeaderWriter, cnr cid.ID, id oid.ID, raw bool) error {