sn/object: add post-initial placement replication by End-rey · Pull Request #3923 · nspcc-dev/neofs-node

End-rey · 2026-04-03T15:29:57Z

Closes #3880.

End-rey · 2026-04-03T15:31:45Z

pkg/services/replicator/replicator.go

 type cfg struct {
 	putTimeout time.Duration
+	queueSize  int
+	workers    int


These parameters can be made configurable if this type of queue is suitable

Meh, we've just removed replicator.pool_size. It'd be nice to not have any configurations, let's have like 8 for now and we'll improve on it later.

no need to have them as cfg fields if they are not configurable. Just use consts

codecov · 2026-04-03T15:33:10Z

Codecov Report

❌ Patch coverage is 46.89655% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.88%. Comparing base (943599a) to head (a4161c8).

Files with missing lines	Patch %	Lines
pkg/services/replicator/queue.go	0.00%	30 Missing ⚠️
pkg/services/object/put/distributed.go	67.12%	17 Missing and 7 partials ⚠️
cmd/neofs-node/object.go	0.00%	17 Missing ⚠️
pkg/services/object/put/ec.go	80.00%	2 Missing and 1 partial ⚠️
pkg/services/replicator/replicator.go	0.00%	2 Missing ⚠️
pkg/services/replicator/task.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3923      +/-   ##
==========================================
+ Coverage   26.82%   26.88%   +0.05%     
==========================================
  Files         670      671       +1     
  Lines       44294    44427     +133     
==========================================
+ Hits        11881    11943      +62     
- Misses      31309    31373      +64     
- Partials     1104     1111       +7

☔ View full report in Codecov by Sentry.
📢 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.

CHANGELOG.md

pkg/services/object/put/distributed.go

pkg/services/replicator/queue.go

pkg/services/replicator/replicator.go

roman-khimov · 2026-04-06T14:47:02Z

pkg/services/replicator/replicator.go

 type cfg struct {
 	putTimeout time.Duration
+	queueSize  int
+	workers    int


Meh, we've just removed replicator.pool_size. It'd be nice to not have any configurations, let's have like 8 for now and we'll improve on it later.

roman-khimov · 2026-04-06T14:50:02Z

pkg/services/object/put/distributed.go

 	}

+	t.replicateRemainingPrimaryNodes(obj, objNodeLists[:len(fullRepRules)], fullRepRules, repProg)
+	t.replicateRemainingECRules(obj, ecRules, objNodeLists[len(fullRepRules):], appliedECRules)


Needs to be checked for various buffer reuses. Although obj is not a problem and objNodeLists should be fine too.

The only thing I found is that replicator.Task now crosses the boundary of an asynchronous call, so it needs to maintain its own list of nodes instead of aliasing caller memory.

cthulhu-rider

partially viewed

cthulhu-rider · 2026-04-10T10:15:46Z

pkg/services/replicator/queue.go

+)
+
+// ErrQueueFull means that a replication task could not be queued immediately.
+var ErrQueueFull = errors.New("replication queue is full")


used inside the package only, why exported?

cthulhu-rider · 2026-04-10T10:17:38Z

pkg/services/replicator/queue.go

+		})
+	}
+
+	<-ctx.Done()


seems excessive, we wait all workes to be interrupted by ctx anyway

cthulhu-rider · 2026-04-10T10:20:36Z

pkg/services/replicator/queue.go

+		case <-ctx.Done():
+			return
+		case task := <-p.taskQueue:
+			res := new(countingReplicationResult)


can be reused b/w iterations

cthulhu-rider · 2026-04-10T10:23:25Z

pkg/services/replicator/task.go

 // SetNodes sets a list of potential object holders.
 func (t *Task) SetNodes(v []netmap.NodeInfo) {
-	t.nodes = v
+	t.nodes = slices.Clone(v)


why cloning is needed?

I thought about how the queue changed Task lifetime: it is no longer executed immediately, so it should own nodes instead of aliasing caller memory.

instead of aliasing caller memory

why not? It does not mutate it

You're right, I don't currently see a place where that slice is mutated after being passed in. I added the clone as a safety measure because Task is now queued and can outlive the caller. If we don't want the extra copy, i can drop it and document that the caller must not modify or reuse the slice after SetNodes.

cthulhu-rider · 2026-04-10T10:26:16Z

pkg/services/replicator/replicator.go


 	localNodeKey LocalNodeKey
+
+	taskQueue chan Task


lets make this a Replicator field. It's configured but initialized internally

cthulhu-rider · 2026-04-10T10:28:43Z

pkg/services/replicator/replicator.go

 type cfg struct {
 	putTimeout time.Duration
+	queueSize  int
+	workers    int


no need to have them as cfg fields if they are not configurable. Just use consts

cthulhu-rider · 2026-04-10T10:30:26Z

cmd/neofs-node/object.go

 		policer.WithBoostMultiplier(c.appCfg.Policer.BoostMultiplier),
 	)

+	c.workers = append(c.workers, c.replicator)


merge into existing append

cthulhu-rider · 2026-04-10T10:35:26Z

pkg/services/object/put/ec.go

 }
+
+func ecNodesForPart(nodeList []netmap.NodeInfo, partIdx, totalParts int) []netmap.NodeInfo {
+	return slices.Collect(func(ni func(netmap.NodeInfo) bool) {


looks trickier than it might be. Just make a slice of len(nodeList) and fill in for

cthulhu-rider · 2026-04-10T10:36:44Z

cmd/neofs-node/object.go

+}
+
+func (r putPostPlacementReplicator) HandlePostPlacement(obj *object.Object, nodes []netmapsdk.NodeInfo) {
+	if len(nodes) == 0 {


it's senseless to me to call this w/ empty node list. Single current caller prevents this on it's own

There were two callers, but I understood the point and checked before making the call.

cthulhu-rider · 2026-04-10T10:43:11Z

pkg/services/object/put/distributed.go

 }

+func (t *distributedTarget) replicateRemainingPrimaryNodes(obj object.Object, nodeLists [][]netmap.NodeInfo, repRules []uint, prog *repProgress) {
+	if t.initialPolicy == nil || t.postPlacementReplicator == nil || len(nodeLists) == 0 || prog == nil {


i wouldn't call this method in these cases at all. Same for EC

btw in which cases t.postPlacementReplicator is nil?

Done.

btw in which cases t.postPlacementReplicator is nil?

Only in the tests, but I fixed that behavior.

Queue post-placement tasks in replicator. Start post-placement replication after successful initial placement. Handle both REP replicas and EC part placement. Add tests. Closes #3880. Signed-off-by: Andrey Butusov <andrey@nspcc.io>

End-rey self-assigned this Apr 3, 2026

End-rey requested review from carpawell, cthulhu-rider and roman-khimov as code owners April 3, 2026 15:30

End-rey commented Apr 3, 2026

View reviewed changes

roman-khimov reviewed Apr 6, 2026

View reviewed changes

End-rey force-pushed the post-initial-replication branch 2 times, most recently from ba50162 to 664367c Compare April 7, 2026 11:40

End-rey requested a review from roman-khimov April 7, 2026 11:42

roman-khimov approved these changes Apr 7, 2026

View reviewed changes

cthulhu-rider reviewed Apr 10, 2026

View reviewed changes

End-rey force-pushed the post-initial-replication branch 2 times, most recently from 14be6c2 to a4161c8 Compare April 10, 2026 15:22

sn/object: optimize post-initial placement replication

a4161c8

Queue post-placement tasks in replicator. Start post-placement replication after successful initial placement. Handle both REP replicas and EC part placement. Add tests. Closes #3880. Signed-off-by: Andrey Butusov <andrey@nspcc.io>

Conversation

End-rey commented Apr 3, 2026

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!

codecov bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

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!

cthulhu-rider left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Apr 3, 2026 •

edited

Loading