feat/Process object notification from chain #3085

carpawell · 2025-01-20T19:17:35Z

No description provided.

codecov · 2025-01-20T19:19:56Z

Codecov Report

Attention: Patch coverage is 29.34783% with 520 lines in your changes missing coverage. Please review.

Project coverage is 23.06%. Comparing base (eb90734) to head (dcbd80d).
Report is 11 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/services/meta/notifications.go	30.67%	253 Missing and 34 partials ⚠️
pkg/services/meta/meta.go	7.81%	118 Missing ⚠️
cmd/neofs-node/meta.go	0.00%	87 Missing ⚠️
pkg/services/meta/container.go	77.00%	17 Missing and 6 partials ⚠️
cmd/neofs-node/config.go	0.00%	4 Missing ⚠️
cmd/neofs-node/main.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3085      +/-   ##
==========================================
+ Coverage   22.93%   23.06%   +0.12%     
==========================================
  Files         750      755       +5     
  Lines       58683    59495     +812     
==========================================
+ Hits        13459    13721     +262     
- Misses      44282    44784     +502     
- Partials      942      990      +48

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

pkg/services/meta/container.go

pkg/services/meta/meta.go

roman-khimov · 2025-01-31T12:37:38Z

pkg/services/meta/notifications.go

+					continue
+				}
+
+				go func() {


Unordered. Potentially dangerous resource-wise.

Unordered

can this be bad? eventually, we plan to receive every notification in a block at once. now the order and new block handling is also not been ordered and will not be

Potentially dangerous resource-wise.

we have been discussing notification handling from time to time for so long. we have to read notifications always as says neo-go client (IMO, it is the client's bad restriction and it is a mistake) and we cannot do any other calls if we are blocked on notification handling. considering all the things i know and all the discussions i had, i may only say that using all the resources we have (routines) is the only way to prevent deadlock, otherwise, there is always a number of notifications that can be received via a channel and their number exceeds any buffer size and it leads to the lock

We care about ordering here. That's exactly the feature we need from this chain, ordered events.

Client is limited by a single pipe transmitting all data, it's like HOL blocking issue if you like. But we can build a reasonable enough pipeline with buffers that will work in most cases and will just reset the connection if it's overflown. Then this boils down to synchronizing with the chain from cold start which is the issue we have to solve anyway.

pkg/services/meta/notifications.go

roman-khimov · 2025-01-31T12:48:09Z

pkg/services/meta/notifications.go

+	if !ok {
+		return res, fmt.Errorf("unexpected object ID stack item: %T", arr[1].Value())
+	}
+	meta, ok := arr[2].Value().([]stackitem.MapElement)


Why not handling it as a proper map (yeah, it only works via Index() API now, but still)? You don't care about element order. I think depending on order is too fragile here.

Why not handling it as a proper map (yeah, it only works via Index() API now, but still)?

if you are talking about calling Index and then pick this item in slice, i do not see any difference. i would like to have Get/Pick as the opposite to the existing Add and know nothing about slice but there is no such method. i do not know why

I think depending on order is too fragile here.

i do not agree. while reading this code, yes, it may seem strange. but we are reading node's code that has received notification. it means every node signed a correct order and number of meta fields. so either the order correct and the node can work with it, or nothing happens at all

anyway, i really would like to have Get from the map type, and agree to use it orderless. but i dont

We have FromStackItem in RPC bindings, probably it'll help us here.

what exact struct do you suggest to use from bindings?

pkg/services/meta/notifications.go

carpawell · 2025-02-05T04:00:04Z

There are still some TODOs that require additional attention but in general, it works. Fixes may be done later or maybe now. Some more tests will be added, currently, they are broken because of new updates.

roman-khimov

s/MTP/MPT/

roman-khimov · 2025-02-06T14:45:32Z

pkg/services/meta/notifications.go

+	}
+
+	// TODO: receive full object header and store its non-MTP parts in
+	//  persistent object storage


In fact you need to save original KVs (MPTfied) as well, MPT is slow for reads (even though it can return the same data).

you mean every KV that i put into MPT should also be placed to the raw storage?

roman-khimov · 2025-02-06T14:45:52Z

pkg/services/meta/notifications.go

+}
+
+func (m *Meta) handleObjectNotification(e objEvent) error {
+	// TODO check network magic?


Can be done.

will do. i could not answer if it is possible to misuse networks. but if we have this field from our chain, i guess, it should be done

It's mostly for replay protection, but we have it, can be checked.

roman-khimov · 2025-02-06T14:47:16Z

cmd/neofs-node/config.go

@@ -698,6 +715,13 @@ func initBasics(c *cfg, key *keys.PrivateKey, stateStorage *state.PersistentStor
 		fatalOnErr(err)
 	}

+	c.shared.wsCli, err = rpcclient.NewWS(c.ctx, addresses[0], rpcclient.WSOptions{


No tries for other addresses?

whoops, rebase problems. this field is not used and it is kinda no-op code, multi endpoints are handled inside meta service now

roman-khimov · 2025-02-06T15:14:24Z

pkg/services/meta/notifications.go

+	return epoch.Int64(), nil
+}
+
+func (m *Meta) handleEpochNotification(e int64) error {


Expired objects are not deleted.

hm, we do not have expiration epoch in meta-from-chain, so we cannot find such cases without object HEADing object. or we have to add it to the contract

Can be postponed.

cthulhu-rider

gr8 work is done, viewed few random parts only

pkg/services/meta/container.go

cmd/neofs-node/meta.go

pkg/services/meta/meta.go

cthulhu-rider · 2025-02-12T10:03:30Z

pkg/services/meta/meta.go

+
+					_, err := st.mpt.Store.PersistSync()
+					if err != nil {
+						return fmt.Errorf("persisting %q storage: %w", st.path, err)


mb worth keep persisting other storages?

it is a big question for me in fact. i feel like either everything should be in sync and ok, or we should stop
@roman-khimov

pkg/services/meta/meta.go

roman-khimov · 2025-02-12T18:23:05Z

pkg/services/meta/container.go

+		newKVs[string(append([]byte{0, previousPartIndex}, commsuffix...))] = e.prevObject
+	}
+	if len(e.deletedObjects) > 0 {
+		newKVs[string(append([]byte{0, deletedIndex}, commsuffix...))] = e.deletedObjects


The object is deleted. Which means you need to delete it from the object index and be done with it.

Information from FS chain is placed inside MPT structures reused from neo-go. Only the latest state is stored. Every container has its own MPT storage based on a separate bolt KV database. Refs #3070. Signed-off-by: Pavel Karpy <[email protected]>

Store caught information in MPT structures. Closes #3070. Signed-off-by: Pavel Karpy <[email protected]>

carpawell force-pushed the feat/process-meta-on-chain branch from 0207dcb to 50a8e8a Compare January 20, 2025 19:25

carpawell mentioned this pull request Jan 20, 2025

Handle meta events #3070

Open

carpawell force-pushed the feat/process-meta-on-chain branch 4 times, most recently from 1510922 to ca811c2 Compare January 31, 2025 03:16

roman-khimov reviewed Jan 31, 2025

View reviewed changes

carpawell force-pushed the feat/process-meta-on-chain branch from ca811c2 to 1e3b86d Compare February 5, 2025 03:57

carpawell marked this pull request as ready for review February 5, 2025 04:00

carpawell requested a review from cthulhu-rider as a code owner February 5, 2025 04:00

carpawell requested a review from roman-khimov February 5, 2025 04:00

roman-khimov reviewed Feb 6, 2025

View reviewed changes

carpawell force-pushed the feat/process-meta-on-chain branch from 1e3b86d to 90e9810 Compare February 10, 2025 02:02

cthulhu-rider reviewed Feb 12, 2025

View reviewed changes

pkg/services/meta/meta.go Outdated Show resolved Hide resolved

roman-khimov reviewed Feb 12, 2025

View reviewed changes

carpawell force-pushed the feat/process-meta-on-chain branch 6 times, most recently from 1f2dae0 to 9d16e91 Compare February 13, 2025 22:42

carpawell requested review from roman-khimov and cthulhu-rider February 13, 2025 22:43

carpawell force-pushed the feat/process-meta-on-chain branch 2 times, most recently from fb07736 to b1073bb Compare February 14, 2025 09:10

node/meta: initial MPT meta storage

2e28ada

Information from FS chain is placed inside MPT structures reused from neo-go. Only the latest state is stored. Every container has its own MPT storage based on a separate bolt KV database. Refs #3070. Signed-off-by: Pavel Karpy <[email protected]>

node/meta: catch meta notifications from chain

dcbd80d

Store caught information in MPT structures. Closes #3070. Signed-off-by: Pavel Karpy <[email protected]>

carpawell force-pushed the feat/process-meta-on-chain branch from b1073bb to dcbd80d Compare February 14, 2025 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/Process object notification from chain #3085

feat/Process object notification from chain #3085

carpawell commented Jan 20, 2025

codecov bot commented Jan 20, 2025 •

edited

Loading

roman-khimov Jan 31, 2025

carpawell Jan 31, 2025

roman-khimov Jan 31, 2025

roman-khimov Jan 31, 2025

carpawell Jan 31, 2025

roman-khimov Jan 31, 2025

carpawell Feb 7, 2025

carpawell commented Feb 5, 2025 •

edited

Loading

roman-khimov left a comment

roman-khimov Feb 6, 2025

carpawell Feb 6, 2025

roman-khimov Feb 6, 2025

roman-khimov Feb 6, 2025

carpawell Feb 6, 2025

roman-khimov Feb 6, 2025

roman-khimov Feb 6, 2025

carpawell Feb 6, 2025

roman-khimov Feb 6, 2025

carpawell Feb 7, 2025

roman-khimov Feb 7, 2025

cthulhu-rider left a comment

cthulhu-rider Feb 12, 2025

carpawell Feb 12, 2025

roman-khimov Feb 12, 2025

feat/Process object notification from chain #3085

Are you sure you want to change the base?

feat/Process object notification from chain #3085

Conversation

carpawell commented Jan 20, 2025

codecov bot commented Jan 20, 2025 • edited Loading

Codecov Report

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 Feb 5, 2025 • edited Loading

roman-khimov left a 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

cthulhu-rider left a 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

codecov bot commented Jan 20, 2025 •

edited

Loading

carpawell commented Feb 5, 2025 •

edited

Loading