sn: do not treat more than a single EC part on the same node as a success by carpawell · Pull Request #4069 · nspcc-dev/neofs-node

carpawell · 2026-07-07T20:21:16Z

No description provided.

codecov · 2026-07-07T20:24:42Z

Codecov Report

❌ Patch coverage is 95.23810% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.56%. Comparing base (facd0db) to head (d1a104e).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/services/object/put/ec.go	96.61%	2 Missing ⚠️
pkg/services/object/put/distributed.go	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4069      +/-   ##
==========================================
+ Coverage   26.45%   26.56%   +0.10%     
==========================================
  Files         671      671              
  Lines       45581    45632      +51     
==========================================
+ Hits        12060    12122      +62     
+ Misses      32390    32383       -7     
+ Partials     1131     1127       -4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

carpawell · 2026-07-08T15:52:03Z

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              1d4h ⚑ ◒  
▶ neofs-cli container create -r s01.neofs.devenv:8080 --policy 'EC 3/1 CBF 1' -w wallets/wallet.json --basic-acl "public-read-write"                           
Enter password > 
neofscontainer creation request accepted for processing (the operation may not be completed yet)
container ID: CaSmiYvN6SuQ8E7bMVzHkzLsMPEB1f9tuZYEXZNuaBFK

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        1y75d § ◒  ⍉
▶ neofs-cli object put -r s01.neofs.devenv:8080 -w wallets/wallet.json --cid CaSmiYvN6SuQ8E7bMVzHkzLsMPEB1f9tuZYEXZNuaBFK --file tmp/10Klines.txt --timeout 1m
Enter password > 
 100000 / 100000 [========================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================] 100.00% 0s
[tmp/10Klines.txt] Object successfully stored
  OID: ExuNBE8ucepxuxHjaCPDKA3B2YMCZBwYPzST5mB9VXM7
  CID: CaSmiYvN6SuQ8E7bMVzHkzLsMPEB1f9tuZYEXZNuaBFK

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         1y75d § ◒  
▶ docker stop s04                                                                                                                                             
s04

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         1y75d § ◒  
▶ neofs-cli object put -r s01.neofs.devenv:8080 -w wallets/wallet.json --cid CaSmiYvN6SuQ8E7bMVzHkzLsMPEB1f9tuZYEXZNuaBFK --file tmp/10Klines.txt --timeout 1m
Enter password > 
 100000 / 100000 [=======================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================] 100.00% 40s
Error: rpc error: finish object stream: status: code = 1024 message = incomplete object PUT by placement: only 3 EC parts was put successfully for 3 data parts number, latest error: save on SN #2: could not close object stream: replicate object to remote node (key=038c862959e56b43e20f79187c4fe9e0bc7c8c66c1603e6cf0ec7f87ab6b08dc35): all 038c862959e56b43e20f79187c4fe9e0bc7c8c66c1603e6cf0ec7f87ab6b08dc35 endpoints failed, first error: /dns4/s04.neofs.devenv/tcp/8080: API transport (op=/neo.fs.v2.object.ObjectService/Replicate): rpc error: code = Unavailable desc = connection error: desc = "transport: Error while dialing: dial tcp 192.168.130.74:8080: i/o timeout"

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        1y75d ⚑ ◒  ⍉
▶ docker stop s03
s03

NSPCC/git/neofs-dev-env  master ✗                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         1y75d ⚑ ◒  
▶ neofs-cli object put -r s01.neofs.devenv:8080 -w wallets/wallet.json --cid CaSmiYvN6SuQ8E7bMVzHkzLsMPEB1f9tuZYEXZNuaBFK --file tmp/10Klines.txt --timeout 1m
Enter password > 
 100000 / 100000 [=======================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================================] 100.00% 20s
Error: rpc error: finish object stream: status: code = 1024 message = not enought nodes for EC parts (2 parts were put)

cthulhu-rider · 2026-07-09T11:25:04Z

-		err := t.applyECRule(t.sessionSigner, obj, ecRuleIdx, payloadParts, objNodeLists[ruleIdx])
+	handleECRule := func(ruleIdx int, ecRuleIdx int, payloadParts [][]byte, ecRule iec.Rule) (bool, error) {
+		var nodes = objNodeLists[ruleIdx]
+		if len(nodes) < int(ecRule.DataPartNum) {


unrelated. And this must never happen

not unrelated to me, this is highly used inside PR's new logic. but if it is always true (nothing is said about it in ContainerNodes interface) then ok, dropped

cthulhu-rider · 2026-07-10T07:18:12Z


-func (t *distributedTarget) applyECRule(signer neofscrypto.Signer, obj object.Object, ruleIdx int, payloadParts [][]byte, nodeList []netmap.NodeInfo) error {
-	var eg errgroup.Group
+var errStopECIter = errors.New("stop EC placement iteration")


seems redundant, it's enough returning bool from submitNodeFailure()

err is more explicit to me that smth is wrong, ok made it bool

cthulhu-rider · 2026-07-10T07:26:24Z

+}
+
+func (p *ecProgress) canTryNode(n netmap.NodeInfo) bool {
+	if p == nil {


checking this by callers is more clear to me

i read it as "no need to track progress" logic, so put it inside ecProgress. moved

cthulhu-rider · 2026-07-10T07:31:37Z

+		return errStopECIter
+	}
+	p.leftAvailableNodes--
+	placementFailed := p.leftAvailableNodes-int(p.ecRule.DataPartNum) < 0


what if there is 1 part left to save, 1 available node is left and policy is 2/1?

cthulhu-rider · 2026-07-10T07:33:38Z

+var errStopECIter = errors.New("stop EC placement iteration")
+
+func newECProgress(nodeList []netmap.NodeInfo, ecRule iec.Rule) *ecProgress {
+	nodes := make(map[string]struct{}, len(nodeList))


nodeList is a set, so it's enough to deal with idxs

Can even be a bit set. And can use atomics instead of locks then.

not sure why you think it is critical in a routine that calculates EC objects, hashes, uses network, disks, etc, but reworked

cthulhu-rider · 2026-07-10T07:35:54Z

+		return fmt.Errorf("not enought nodes for EC parts (%d parts were put)", successes)
+	}
+	err.singleErr = fmt.Errorf("only %d EC parts were put successfully"+
+		" for %d/%d EC rule, latest error: %w", successes, p.ecRule.DataPartNum, p.ecRule.ParityPartNum, err.singleErr)


Suggested change

" for %d/%d EC rule, latest error: %w", successes, p.ecRule.DataPartNum, p.ecRule.ParityPartNum, err.singleErr)

" for %s EC rule, latest error: %w", successes, p.ecRule, err.singleErr)

cthulhu-rider · 2026-07-10T07:36:24Z

+
+	var successes = p.successPuts.Load()
+	if p.stop {
+		return fmt.Errorf("not enought nodes for EC parts (%d parts were put)", successes)


Suggested change

return fmt.Errorf("not enought nodes for EC parts (%d parts were put)", successes)

return fmt.Errorf("not enough nodes for EC parts (%d parts were put)", successes)

cthulhu-rider · 2026-07-10T07:54:13Z

+	if p.stop {
+		return fmt.Errorf("not enought nodes for EC parts (%d parts were put)", successes)
+	}
+	err.singleErr = fmt.Errorf("only %d EC parts were put successfully"+


there can be several rules with same X/Y parameters. Worth adding rule number

roman-khimov · 2026-07-10T10:21:22Z

+var errStopECIter = errors.New("stop EC placement iteration")
+
+func newECProgress(nodeList []netmap.NodeInfo, ecRule iec.Rule) *ecProgress {
+	nodes := make(map[string]struct{}, len(nodeList))


Can even be a bit set. And can use atomics instead of locks then.

Closes #3994. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>

cthulhu-rider · 2026-07-10T15:30:12Z

+		return false
+	}
+	p.failedPuts++
+	placementFailed := len(p.nodes)-p.failedPuts < int(p.ecRule.DataPartNum)


EC 1/1 CBF 1, i.e. 2 nodes

1st node fails, 1 node is left

we cannot put 2 parts, so it's a complete fail. But condition will be false

carpawell self-assigned this Jul 7, 2026

carpawell changed the title ~~sn: do not treat more that single EC part on the same node as success~~ sn: do not treat more than a single EC part on the same node as a success Jul 7, 2026

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch from ab98864 to 551f29e Compare July 7, 2026 20:32

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch from 551f29e to b707499 Compare July 8, 2026 17:32

carpawell marked this pull request as ready for review July 8, 2026 17:32

carpawell requested review from End-rey, cthulhu-rider and roman-khimov as code owners July 8, 2026 17:32

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch 3 times, most recently from c1425f3 to 9c3418c Compare July 8, 2026 19:18

carpawell marked this pull request as draft July 8, 2026 19:18

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch 2 times, most recently from cc042cd to 3662963 Compare July 8, 2026 19:35

carpawell marked this pull request as ready for review July 8, 2026 19:38

carpawell marked this pull request as draft July 8, 2026 19:44

carpawell marked this pull request as ready for review July 8, 2026 19:45

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch from b8bdfbe to 3662963 Compare July 8, 2026 19:56

cthulhu-rider reviewed Jul 10, 2026

View reviewed changes

roman-khimov reviewed Jul 10, 2026

View reviewed changes

sn: do not treat more that single EC part on the same node as success

d1a104e

Closes #3994. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>

carpawell force-pushed the fix/EC-placement-must-support-at-least-DATAPART-number-of-successes branch from 3662963 to d1a104e Compare July 10, 2026 13:35

carpawell requested review from cthulhu-rider and roman-khimov July 10, 2026 13:48

roman-khimov approved these changes Jul 10, 2026

View reviewed changes

Comment thread pkg/services/object/put/ec.go

cthulhu-rider reviewed Jul 10, 2026

View reviewed changes

	" for %d/%d EC rule, latest error: %w", successes, p.ecRule.DataPartNum, p.ecRule.ParityPartNum, err.singleErr)
	" for %s EC rule, latest error: %w", successes, p.ecRule, err.singleErr)

	return fmt.Errorf("not enought nodes for EC parts (%d parts were put)", successes)
	return fmt.Errorf("not enough nodes for EC parts (%d parts were put)", successes)

Uh oh!

Conversation

carpawell commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

carpawell commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carpawell commented Jul 7, 2026 •

edited

Loading

codecov Bot commented Jul 7, 2026 •

edited

Loading

carpawell commented Jul 8, 2026 •

edited

Loading