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

Drop compatibility code from loadPlacementBuilder.BuildPlacement #2456

Closed
roman-khimov opened this issue Jul 25, 2023 · 8 comments
Closed

Drop compatibility code from loadPlacementBuilder.BuildPlacement #2456

roman-khimov opened this issue Jul 25, 2023 · 8 comments
Assignees
Labels
neofs-storage Storage node application issues
Milestone

Comments

@roman-khimov
Copy link
Member

Expected Behavior

Placements are provided by SDK.

Current Behavior

We had to replicate the old behavior in loadPlacementBuilder.BuildPlacement when updating to RC8 (#2346).

Possible Solution

We will be updating to RC10 and when that happens we'll have new sorting everywhere. So we can drop this code at the same time and either don't use placement there at all or generate some OID for it as needed.

Refs. nspcc-dev/neofs-sdk-go#438 and #2442.

@roman-khimov roman-khimov added neofs-storage Storage node application issues refactor labels Jul 25, 2023
@roman-khimov roman-khimov added this to the v0.39.0 milestone Jul 25, 2023
@roman-khimov roman-khimov modified the milestones: v0.39.0, v0.38.0 Aug 4, 2023
@cthulhu-rider
Copy link
Contributor

i see two ways:

  1. calc fake oid.ID with load_announcement_<epoch>0000... with fixed-len epoch
  2. sort nodes by public keys lexicographically

@carpawell
Copy link
Member

  1. you mean the same "main" node for the same container set every epoch?

@cthulhu-rider
Copy link
Contributor

  1. you mean the same "main" node for the same container set every epoch?

yes, why not (taking into account many containers with different leaders). We can also rotate leader by epoch, but this doesn't make much difference

@carpawell
Copy link
Member

If we need to have it fine with RC10 I would go that way (the same node subset choose a different node for a different container).

Also, I am totally ok with a separate func in SDK (does not apply any pivot at all, only the epoch).

yes, why not

Not good for a decentralized network IMO. Predefined node does all the money work.

@cthulhu-rider
Copy link
Contributor

cthulhu-rider commented Aug 4, 2023

Not good for a decentralized network IMO

if u focus on decentralization, then each node will act for itself and send own values directly to the contract instead of other untrusted node (rotation changes nothing in terms of decentralization)

in total, storage nodes' anarchy is not good for economic model, imo we should do some sort of audit actively from the Inner Ring

but this is about other issue, i proposed technically working alternatives

@roman-khimov
Copy link
Member Author

each node will act for itself and send own values directly to the contract instead of other untrusted node (rotation changes nothing in terms of decentralization)

That's the way it should be. But long-term. Short-term we need to rotate leaders in some predictable manner.

@carpawell carpawell self-assigned this Aug 7, 2023
cthulhu-rider added a commit that referenced this issue Aug 8, 2023
According to #2456, custom placement tests are no longer needed.

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider added a commit that referenced this issue Aug 8, 2023
According to #2456, custom placement tests are no longer needed.

Signed-off-by: Leonard Lyubich <[email protected]>
@roman-khimov
Copy link
Member Author

Fixed in #2478 without a reference?

@carpawell
Copy link
Member

carpawell commented Aug 15, 2023

Fixed in #2478 without a reference?

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-storage Storage node application issues
Projects
None yet
Development

No branches or pull requests

3 participants