Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netmap: Return same type from reading methods #273

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Sep 28, 2022

I've changed format of the Netmap contract storage - should I do some compatibility actions? @fyrchik

)

// Node groups data related to NeoFS storage nodes registered in the NeoFS
// network. The information is stored in the current contract.
type Node struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're planning to update contract in the existing chain, then you need to write a migration routine. It should migrate the old netmap contract storage scheme to the new one (storageNode -> Node transformation), otherwise contract won't be able to work with the previously saved data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, migration job should be performed, thanks.

I've started from documenting (for developers) the storage structure #276.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've decided to postpone the migration #277.

switch state {
case NodeStateOffline:
removeFromNetmap(ctx, publicKey)
runtime.Log("remove storage node from the network map")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need these logs? There's a notification in the end of updateCandidateState, it should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe to distingush notification losses from non-throws. I don't mind to have such log messages.

Copy link
Contributor

@fyrchik fyrchik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for migration

@cthulhu-rider cthulhu-rider marked this pull request as ready for review September 30, 2022 11:58
@cthulhu-rider cthulhu-rider force-pushed the refactor/netmap branch 4 times, most recently from 8260152 to 425b08f Compare September 30, 2022 13:03
Leonard Lyubich and others added 2 commits September 30, 2022 17:05
There is a need to return similar structure of information about the
storage nodes from the contract storage readers. In previous
implementation some methods didn't return node state which can differ
with the one encoded in the node's BLOB.

Define `Node` structure of the information about the storage nodes
recorded in the contract storage. Return `[]Node` from all related
methods.

Also improve docs of touched contract methods.

Signed-off-by: Leonard Lyubich <[email protected]>
Call changelog section with unreleased changes `Unreleased`. Also make
it as a diff link.

Signed-off-by: Leonard Lyubich <[email protected]>
@cthulhu-rider cthulhu-rider merged commit f276e24 into nspcc-dev:master Sep 30, 2022
@cthulhu-rider cthulhu-rider deleted the refactor/netmap branch September 30, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation netmap Netmap contract related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants