Skip to content

Commit

Permalink
netmap/policy: Replace adders with setters for list types
Browse files Browse the repository at this point in the history
Еhe need to expand a partially or fully formed storage policy is either
absent or fully covered by slice manipulations. The parameter
variability is also excessive: by default the list is empty and
resetting it by calling it without parameters is difficult to read.
Adding elements one-by-one is inefficient in terms of resources.

While this is breaking change, it forces more clear app code and
performance.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
  • Loading branch information
cthulhu-rider committed Apr 12, 2024
1 parent fd839ed commit d58930b
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 43 deletions.
2 changes: 1 addition & 1 deletion client/container_statistic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func prepareContainer(accountID user.ID) container.Container {
rd.SetNumberOfObjects(1)

pp.SetContainerBackupFactor(1)
pp.AddReplicas(rd)
pp.SetReplicas([]netmap.ReplicaDescriptor{rd})
cont.SetPlacementPolicy(pp)

return cont
Expand Down
6 changes: 3 additions & 3 deletions container/container_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ func TestContainer_CopyTo(t *testing.T) {
var rd netmap.ReplicaDescriptor
rd.SetSelectorName("selector")
rd.SetNumberOfObjects(100)
pp.AddReplicas(rd)
pp.SetReplicas([]netmap.ReplicaDescriptor{rd})

var f netmap.Filter
f.SetName("filter")
pp.AddFilters(f)
pp.SetFilters([]netmap.Filter{f})

var s netmap.Selector
s.SetName("selector")
pp.AddSelectors(s)
pp.SetSelectors([]netmap.Selector{s})

container.SetPlacementPolicy(pp)

Expand Down
2 changes: 1 addition & 1 deletion container/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func ExampleContainer_Init() {
// placement policy and replicas definition is required
var pp netmap.PlacementPolicy
pp.SetContainerBackupFactor(1)
pp.AddReplicas(rd)
pp.SetReplicas([]netmap.ReplicaDescriptor{rd})

cnr.SetPlacementPolicy(pp)
}
Expand Down
6 changes: 3 additions & 3 deletions netmap/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ func newSelector(name string, attr string, count uint32, filter string, clause f

func newPlacementPolicy(bf uint32, rs []ReplicaDescriptor, ss []Selector, fs []Filter) (p PlacementPolicy) {
p.SetContainerBackupFactor(bf)
p.AddReplicas(rs...)
p.AddSelectors(ss...)
p.AddFilters(fs...)
p.SetReplicas(rs)
p.SetSelectors(ss)
p.SetFilters(fs)
return p
}

Expand Down
40 changes: 17 additions & 23 deletions netmap/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,23 +234,21 @@ func (r ReplicaDescriptor) SelectorName() string {
return r.m.GetSelector()
}

// AddReplicas adds a bunch object replica's characteristics.
// SetReplicas sets list of object replica's characteristics.
//
// See also [PlacementPolicy.Replicas], [PlacementPolicy.NumberOfReplicas],
// [PlacementPolicy.ReplicaNumberByIndex].
func (p *PlacementPolicy) AddReplicas(rs ...ReplicaDescriptor) {
off := len(p.replicas)

p.replicas = append(p.replicas, make([]netmap.Replica, len(rs))...)
func (p *PlacementPolicy) SetReplicas(rs []ReplicaDescriptor) {
p.replicas = make([]netmap.Replica, len(rs))

for i := range rs {
p.replicas[off+i] = rs[i].m
p.replicas[i] = rs[i].m
}
}

// Replicas returns list of object replica characteristics.
//
// See also [PlacementPolicy.AddReplicas], [PlacementPolicy.NumberOfReplicas],
// See also [PlacementPolicy.SetReplicas], [PlacementPolicy.NumberOfReplicas],
// [PlacementPolicy.ReplicaNumberByIndex].
func (p PlacementPolicy) Replicas() []ReplicaDescriptor {
rs := make([]ReplicaDescriptor, len(p.replicas))
Expand All @@ -260,7 +258,7 @@ func (p PlacementPolicy) Replicas() []ReplicaDescriptor {
return rs
}

// NumberOfReplicas returns number of replica descriptors set using AddReplicas.
// NumberOfReplicas returns number of replica descriptors set using SetReplicas.
//
// Zero PlacementPolicy has no replicas which is incorrect according to the
// NeoFS API protocol.
Expand Down Expand Up @@ -408,19 +406,17 @@ func (s *Selector) FilterName() string {
return s.m.GetFilter()
}

// AddSelectors adds a Selector bunch to form the subset of the nodes
// to store container objects.
// SetSelectors sets list of Selector to form the subset of the nodes to store
// container objects.
//
// Zero PlacementPolicy does not declare selectors.
//
// See also [PlacementPolicy.Selectors].
func (p *PlacementPolicy) AddSelectors(ss ...Selector) {
off := len(p.selectors)

p.selectors = append(p.selectors, make([]netmap.Selector, len(ss))...)
func (p *PlacementPolicy) SetSelectors(ss []Selector) {
p.selectors = make([]netmap.Selector, len(ss))

for i := range ss {
p.selectors[off+i] = ss[i].m
p.selectors[i] = ss[i].m
}
}

Expand All @@ -429,7 +425,7 @@ func (p *PlacementPolicy) AddSelectors(ss ...Selector) {
//
// Zero PlacementPolicy does not declare selectors.
//
// See also [PlacementPolicy.AddSelectors].
// See also [PlacementPolicy.SetSelectors].
func (p PlacementPolicy) Selectors() []Selector {
ss := make([]Selector, len(p.selectors))
for i := range p.selectors {
Expand Down Expand Up @@ -582,7 +578,7 @@ func (x *Filter) LogicalAND(filters ...Filter) {
//
// Zero PlacementPolicy has no filters.
//
// See also [PlacementPolicy.AddFilters].
// See also [PlacementPolicy.SetFilters].
func (p PlacementPolicy) Filters() []Filter {
fs := make([]Filter, len(p.filters))
for i := range p.filters {
Expand All @@ -591,18 +587,16 @@ func (p PlacementPolicy) Filters() []Filter {
return fs
}

// AddFilters adds a Filter bunch that will be applied when selecting nodes.
// SetFilters sets list of Filter that will be applied when selecting nodes.
//
// Zero PlacementPolicy has no filters.
//
// See also [PlacementPolicy.Filters].
func (p *PlacementPolicy) AddFilters(fs ...Filter) {
off := len(p.filters)

p.filters = append(p.filters, make([]netmap.Filter, len(fs))...)
func (p *PlacementPolicy) SetFilters(fs []Filter) {
p.filters = make([]netmap.Filter, len(fs))

for i := range fs {
p.filters[off+i] = fs[i].m
p.filters[i] = fs[i].m
}
}

Expand Down
8 changes: 4 additions & 4 deletions netmap/policy_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ func TestPlacementPolicy_CopyTo(t *testing.T) {
var rd ReplicaDescriptor
rd.SetSelectorName("selector")
rd.SetNumberOfObjects(100)
pp.AddReplicas(rd)
pp.SetReplicas([]ReplicaDescriptor{rd})

var f Filter
f.SetName("filter")
pp.AddFilters(f)
pp.SetFilters([]Filter{f})

var s Selector
s.SetName("selector")
pp.AddSelectors(s)
pp.SetSelectors([]Selector{s})

t.Run("copy", func(t *testing.T) {
var dst PlacementPolicy
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestPlacementPolicy_CopyTo(t *testing.T) {
topFilter.setInnerFilters(netmap.EQ, []Filter{includedFilter})

var policy PlacementPolicy
policy.AddFilters(topFilter)
policy.SetFilters([]Filter{topFilter})

var dst PlacementPolicy
policy.CopyTo(&dst)
Expand Down
6 changes: 3 additions & 3 deletions netmap/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestPlacementPolicy_Replicas(t *testing.T) {
r.SetNumberOfObjects(2)
rs = append(rs, r)

p.AddReplicas(rs...)
p.SetReplicas(rs)
require.Equal(t, rs, p.Replicas())

var m netmapv2.PlacementPolicy
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestPlacementPolicy_Selectors(t *testing.T) {
s.SelectDistinct()
ss = append(ss, s)

p.AddSelectors(ss...)
p.SetSelectors(ss)
require.Equal(t, ss, p.Selectors())

var m netmapv2.PlacementPolicy
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestPlacementPolicy_Filters(t *testing.T) {
f.LogicalAND(sub1, sub2)
fs = append(fs, f)

p.AddFilters(fs...)
p.SetFilters(fs)
require.Equal(t, fs, p.Filters())

var m netmapv2.PlacementPolicy
Expand Down
6 changes: 3 additions & 3 deletions netmap/test/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ func Selector() (x netmap.Selector) {
// PlacementPolicy returns random netmap.PlacementPolicy.
func PlacementPolicy() (p netmap.PlacementPolicy) {
p.SetContainerBackupFactor(9)
p.AddFilters(Filter(), Filter())
p.AddReplicas(Replica(), Replica())
p.AddSelectors(Selector(), Selector())
p.SetFilters([]netmap.Filter{Filter(), Filter()})
p.SetReplicas([]netmap.ReplicaDescriptor{Replica(), Replica()})
p.SetSelectors([]netmap.Selector{Selector(), Selector()})

return
}
Expand Down
2 changes: 1 addition & 1 deletion pool/pool_aio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ func testCreateContainer(ctx context.Context, t *testing.T, signer neofscrypto.S

var pp netmap.PlacementPolicy
pp.SetContainerBackupFactor(1)
pp.AddReplicas(rd)
pp.SetReplicas(rd)

Check failure on line 726 in pool/pool_aio_test.go

View workflow job for this annotation

GitHub Actions / Coverage

cannot use rd (variable of type "github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor) as []"github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor value in argument to pp.SetReplicas

Check failure on line 726 in pool/pool_aio_test.go

View workflow job for this annotation

GitHub Actions / Tests (ubuntu-latest, 1.20)

cannot use rd (variable of type "github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor) as []"github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor value in argument to pp.SetReplicas

Check failure on line 726 in pool/pool_aio_test.go

View workflow job for this annotation

GitHub Actions / Tests (ubuntu-latest, 1.21)

cannot use rd (variable of type "github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor) as []"github.com/nspcc-dev/neofs-sdk-go/netmap".ReplicaDescriptor value in argument to pp.SetReplicas

cont.SetPlacementPolicy(pp)

Expand Down
2 changes: 1 addition & 1 deletion waiter/example_waiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func ExampleNewWaiter() {
// prepare placement policy.
pp.SetContainerBackupFactor(1)
rd.SetNumberOfObjects(1)
pp.AddReplicas(rd)
pp.SetReplicas([]netmap.ReplicaDescriptor{rd})
cont.SetPlacementPolicy(pp)

// prepare pool.
Expand Down

0 comments on commit d58930b

Please sign in to comment.