Use new API for background replication #2674

cthulhu-rider · 2023-12-11T17:00:07Z

based on and blocked by client: Add several replication improvements neofs-sdk-go#565

replication demux inside PUT is planned for the next PR

codecov · 2023-12-13T13:54:33Z

Codecov Report

Attention: Patch coverage is 39.73941% with 185 lines in your changes are missing coverage. Please review.

Project coverage is 22.13%. Comparing base (c6a3201) to head (5ba2dea).

Files	Patch %	Lines
pkg/services/object/put/local.go	0.00%	65 Missing ⚠️
pkg/network/transport/object/grpc/replication.go	72.00%	27 Missing and 8 partials ⚠️
cmd/neofs-node/grpc.go	0.00%	19 Missing ⚠️
cmd/neofs-node/object.go	0.00%	18 Missing ⚠️
pkg/services/object/put/remote.go	0.00%	15 Missing ⚠️
pkg/services/replicator/process.go	0.00%	15 Missing ⚠️
pkg/network/cache/multi.go	0.00%	12 Missing ⚠️
cmd/neofs-node/policy.go	82.85%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2674      +/-   ##
==========================================
+ Coverage   22.00%   22.13%   +0.12%     
==========================================
  Files         791      793       +2     
  Lines       47297    47586     +289     
==========================================
+ Hits        10410    10535     +125     
- Misses      35993    36147     +154     
- Partials      894      904      +10

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

cmd/neofs-node/grpc.go

carpawell

Could you point me to the place where ACLs are checked?

pkg/local_object_storage/blobstor/fstree/fstree.go

cmd/neofs-node/replication.go

pkg/network/transport/object/grpc/replication.go

pkg/network/transport/object/grpc/service.go

pkg/local_object_storage/blobstor/common/storage.go

cthulhu-rider · 2024-03-05T10:39:57Z

nspcc-dev/neofs-sdk-go@3f513ed is really desired. But we have current problem with SDK upgrades. So we should either w8 4 #2716 to be able to test this in master or postpone signature optimization

cmd/neofs-node/grpc.go

cmd/neofs-node/policy.go

pkg/network/transport/object/grpc/replication.go

pkg/network/transport/object/grpc/service.go

pkg/network/transport/object/grpc/replication.go

carpawell · 2024-03-27T20:12:35Z

cmd/neofs-node/grpc.go

+				maxRecvSize, math.MaxInt))
+		}
+		maxRecvMsgSizeOpt = grpc.MaxRecvMsgSize(int(maxRecvSize))
+		c.log.Debug("limit max recv gRPC message size to fit max stored objects",


can be info, imo

can ofc, do or not?

I'd leave it to Debug for now.

cmd/neofs-node/policy.go

carpawell · 2024-03-27T20:43:51Z

pkg/network/transport/object/grpc/replication.go

+	switch req.Signature.Scheme {
+	// other cases already checked above
+	case refs.SignatureScheme_ECDSA_SHA512:
+		pubKey = new(neofsecdsa.PublicKey)


we can have some generic func here, since this parsing routine is the same for all the keys (even err message is the same). not needed to have 3 cases then, all can be done in a single branch

this parsing routine is the same for all the keys (even err message is the same)

these are different cases, looks similar (now) - yeah, but not the same. I dont think separate function will make code easier to understand

but how can they differ?

response (message, status details) can depend on particular scheme. The fact that i've made them similar doesnt mean all branches are the same. Current code shows what differs (overall) for different schemes - public key implementation and response messages

doesnt mean all branches are the same

but all branches are the same now

carpawell · 2024-03-27T20:52:03Z

pkg/network/transport/object/grpc/replication.go

+	}
+
+	var clientInCnr, serverInCnr bool
+	err = s.node.ForEachContainerNodePublicKeyInLastTwoEpochs(cnr, func(pubKey []byte) bool {


that is the only call, do we need this interface to be that complex? can it just answers if some key(s) belons to a container in the last two epochs?

this'd have inconvenient interface like M(cid.ID, [][]byte) ([]bool, error). Current method is more reusable and clear to me

well, you added two methods to implement a single functionality (and the first one is used to make the second one usable). it is undesired complexity to me

more reusable

well, not sure it'll be reused ever (just iterate container keys? why?)

but generally not that critical

just iterate container keys? why?

key-container relationship is a widely demanded operation in NeoFS

key-container relationship

this is a "bool" relation, my thread is about it

pkg/services/replicator/process.go

carpawell · 2024-03-29T15:01:18Z

Some conflicts also.

cthulhu-rider · 2024-04-01T15:42:30Z

rebased, cut placement caches and checked still works. Left several refactor issues for the future. All rdy for merge

NeoFS protocol has been recently extended with new object replication RPC `ObjectService.Replicate` separated from the general-purpose `ObjectService.Put` one. According to API of the new RPC, all physically stored objects are transmitted in one message. Also, replication request and response formats are much simpler than for other operations. Serve new RPC by the storage node app. Requests are served similar to `ObjectService.Put` ones with TTL=1 (local only). Refs #2317. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

It's more lightweight and supports binary copying without additional decode-encode round. Based on the fact that if the object is fixed, the request remains unchanged. According to this, transport message is encoded once and sent to all nodes. Closes #2317. Refs #2316. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

carpawell · 2024-04-03T09:59:53Z

pkg/network/transport/object/grpc/replication.go

+		}}, nil
+	} else if !serverInCnr {
+		return &objectGRPC.ReplicateResponse{Status: &status.Status{
+			Code: codeAccessDenied, Message: "server does not match the object's storage policy",


why not line per field?

cthulhu-rider force-pushed the feature/object-separate-replication branch 2 times, most recently from 98848f5 to 450912e Compare December 13, 2023 13:52

cthulhu-rider force-pushed the feature/object-separate-replication branch 7 times, most recently from 0b915af to 3a95318 Compare December 15, 2023 13:25

roman-khimov reviewed Dec 15, 2023

View reviewed changes

cmd/neofs-node/grpc.go Show resolved Hide resolved

carpawell reviewed Dec 18, 2023

View reviewed changes

roman-khimov mentioned this pull request Jan 23, 2024

Replication API extension #2728

Open

cthulhu-rider force-pushed the feature/object-separate-replication branch 5 times, most recently from d1aef55 to 152b8a6 Compare January 30, 2024 16:08

cthulhu-rider force-pushed the feature/object-separate-replication branch from 152b8a6 to 0a8d61a Compare March 5, 2024 10:29

cthulhu-rider changed the title ~~New object replication~~ Use new API for background replication Mar 5, 2024

cthulhu-rider force-pushed the feature/object-separate-replication branch from 0a8d61a to 2317ec8 Compare March 5, 2024 10:33

cthulhu-rider requested review from roman-khimov and carpawell March 5, 2024 10:34

cthulhu-rider marked this pull request as ready for review March 5, 2024 10:50

cthulhu-rider added the blocked Can't be done because of something label Mar 5, 2024

cthulhu-rider mentioned this pull request Mar 5, 2024

Share functionality of placement building between components #1803

Open

roman-khimov removed the blocked Can't be done because of something label Mar 21, 2024

cthulhu-rider force-pushed the feature/object-separate-replication branch from 2317ec8 to fbac66d Compare March 22, 2024 11:20

roman-khimov reviewed Mar 26, 2024

View reviewed changes

carpawell reviewed Mar 27, 2024

View reviewed changes

cthulhu-rider force-pushed the feature/object-separate-replication branch from fbac66d to ecb3a89 Compare March 28, 2024 08:19

cthulhu-rider force-pushed the feature/object-separate-replication branch 2 times, most recently from a657f79 to 6e935b8 Compare April 1, 2024 15:23

This was referenced Apr 1, 2024

Create and use helpers for crypto verifications #2795

Open

Split verification and storage code for incoming objects #2796

Open

carpawell mentioned this pull request Apr 1, 2024

Fix/v2 split placement to non-container node #2792

Merged

roman-khimov mentioned this pull request Apr 2, 2024

Rework network configuration parameters nspcc-dev/neofs-contract#372

Open

roman-khimov approved these changes Apr 2, 2024

View reviewed changes

cthulhu-rider requested a review from carpawell April 3, 2024 06:44

cthulhu-rider added 2 commits April 3, 2024 10:46

cthulhu-rider force-pushed the feature/object-separate-replication branch from 6e935b8 to 5ba2dea Compare April 3, 2024 06:46

carpawell reviewed Apr 3, 2024

View reviewed changes

carpawell approved these changes Apr 3, 2024

View reviewed changes

roman-khimov merged commit 51fefac into master Apr 3, 2024
15 of 16 checks passed

roman-khimov deleted the feature/object-separate-replication branch April 3, 2024 10:31

carpawell mentioned this pull request Apr 12, 2024

object/search: Refactor storage policy processing #2806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new API for background replication #2674

Use new API for background replication #2674

cthulhu-rider commented Dec 11, 2023 •

edited

Loading

codecov bot commented Dec 13, 2023 •

edited

Loading

carpawell left a comment

cthulhu-rider commented Mar 5, 2024 •

edited

Loading

carpawell Mar 27, 2024

cthulhu-rider Mar 28, 2024

roman-khimov Mar 28, 2024

carpawell Mar 27, 2024

cthulhu-rider Mar 28, 2024

carpawell Mar 29, 2024

cthulhu-rider Mar 29, 2024

carpawell Apr 3, 2024

carpawell Mar 27, 2024

cthulhu-rider Mar 28, 2024

carpawell Mar 29, 2024

cthulhu-rider Mar 29, 2024

carpawell Apr 3, 2024

carpawell commented Mar 29, 2024

cthulhu-rider commented Apr 1, 2024

carpawell Apr 3, 2024

Use new API for background replication #2674

Use new API for background replication #2674

Conversation

cthulhu-rider commented Dec 11, 2023 • edited Loading

codecov bot commented Dec 13, 2023 • edited Loading

Codecov Report

carpawell left a comment

Choose a reason for hiding this comment

cthulhu-rider commented Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell commented Mar 29, 2024

cthulhu-rider commented Apr 1, 2024

Choose a reason for hiding this comment

cthulhu-rider commented Dec 11, 2023 •

edited

Loading

codecov bot commented Dec 13, 2023 •

edited

Loading

cthulhu-rider commented Mar 5, 2024 •

edited

Loading