satellite/nodeselection: remove AutoExcludeSubnet filter
It's statefull, therefore it can hit naive users. (NodeFilters couldn't be reused for more than one iterations). But looks like we don't need it, as `SelectBySubnet` doest the same job. Change-Id: Ie85b7f9c2bd9a47293f4e3b359f8b619215c7649
This commit is contained in:
parent
db13e3ef15
commit
5522423871
@ -45,6 +45,13 @@ func GetAnnotation(filter NodeFilter, name string) string {
|
||||
if annotated, ok := filter.(AnnotatedNodeFilter); ok {
|
||||
return annotated.Annotations[name]
|
||||
}
|
||||
if filters, ok := filter.(NodeFilters); ok {
|
||||
for _, filter := range filters {
|
||||
if annotated, ok := filter.(AnnotatedNodeFilter); ok {
|
||||
return annotated.Annotations[name]
|
||||
}
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
@ -82,11 +89,6 @@ func (n NodeFilters) WithCountryFilter(permit location.Set) NodeFilters {
|
||||
return append(n, NewCountryFilter(permit))
|
||||
}
|
||||
|
||||
// WithAutoExcludeSubnets is a helper to create a new filter with additional AutoExcludeSubnets.
|
||||
func (n NodeFilters) WithAutoExcludeSubnets() NodeFilters {
|
||||
return append(n, NewAutoExcludeSubnets())
|
||||
}
|
||||
|
||||
// WithExcludedIDs is a helper to create a new filter with additional WithExcludedIDs.
|
||||
func (n NodeFilters) WithExcludedIDs(ds []storj.NodeID) NodeFilters {
|
||||
return append(n, ExcludedIDs(ds))
|
||||
@ -113,32 +115,6 @@ func (p *CountryFilter) MatchInclude(node *SelectedNode) bool {
|
||||
|
||||
var _ NodeFilter = &CountryFilter{}
|
||||
|
||||
// AutoExcludeSubnets pick at most one node from network.
|
||||
//
|
||||
// Stateful!!! should be re-created for each new selection request.
|
||||
// It should only be used as the last filter.
|
||||
type AutoExcludeSubnets struct {
|
||||
seenSubnets map[string]struct{}
|
||||
}
|
||||
|
||||
// NewAutoExcludeSubnets creates an initialized AutoExcludeSubnets.
|
||||
func NewAutoExcludeSubnets() *AutoExcludeSubnets {
|
||||
return &AutoExcludeSubnets{
|
||||
seenSubnets: map[string]struct{}{},
|
||||
}
|
||||
}
|
||||
|
||||
// MatchInclude implements NodeFilter interface.
|
||||
func (a *AutoExcludeSubnets) MatchInclude(node *SelectedNode) bool {
|
||||
if _, found := a.seenSubnets[node.LastNet]; found {
|
||||
return false
|
||||
}
|
||||
a.seenSubnets[node.LastNet] = struct{}{}
|
||||
return true
|
||||
}
|
||||
|
||||
var _ NodeFilter = &AutoExcludeSubnets{}
|
||||
|
||||
// ExcludedNetworks will exclude nodes with specified networks.
|
||||
type ExcludedNetworks []string
|
||||
|
||||
@ -154,6 +130,21 @@ func (e ExcludedNetworks) MatchInclude(node *SelectedNode) bool {
|
||||
|
||||
var _ NodeFilter = ExcludedNetworks{}
|
||||
|
||||
// ExcludedNodeNetworks exclude nodes which has same net as the one of the specified.
|
||||
type ExcludedNodeNetworks []*SelectedNode
|
||||
|
||||
// MatchInclude implements NodeFilter interface.
|
||||
func (e ExcludedNodeNetworks) MatchInclude(node *SelectedNode) bool {
|
||||
for _, n := range e {
|
||||
if node.LastNet == n.LastNet {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
var _ NodeFilter = ExcludedNodeNetworks{}
|
||||
|
||||
// ExcludedIDs can blacklist NodeIDs.
|
||||
type ExcludedIDs []storj.NodeID
|
||||
|
||||
|
@ -17,23 +17,6 @@ import (
|
||||
"storj.io/common/testrand"
|
||||
)
|
||||
|
||||
func TestNodeFilter_AutoExcludeSubnet(t *testing.T) {
|
||||
|
||||
criteria := NodeFilters{}.WithAutoExcludeSubnets()
|
||||
|
||||
assert.True(t, criteria.MatchInclude(&SelectedNode{
|
||||
LastNet: "192.168.0.1",
|
||||
}))
|
||||
|
||||
assert.False(t, criteria.MatchInclude(&SelectedNode{
|
||||
LastNet: "192.168.0.1",
|
||||
}))
|
||||
|
||||
assert.True(t, criteria.MatchInclude(&SelectedNode{
|
||||
LastNet: "192.168.1.1",
|
||||
}))
|
||||
}
|
||||
|
||||
func TestCriteria_ExcludeNodeID(t *testing.T) {
|
||||
included := testrand.NodeID()
|
||||
excluded := testrand.NodeID()
|
||||
@ -47,29 +30,29 @@ func TestCriteria_ExcludeNodeID(t *testing.T) {
|
||||
assert.True(t, criteria.MatchInclude(&SelectedNode{
|
||||
ID: included,
|
||||
}))
|
||||
|
||||
}
|
||||
|
||||
func TestCriteria_NodeIDAndSubnet(t *testing.T) {
|
||||
excluded := testrand.NodeID()
|
||||
func TestCriteria_ExcludedNodeNetworks(t *testing.T) {
|
||||
criteria := NodeFilters{}
|
||||
criteria = append(criteria, ExcludedNodeNetworks{
|
||||
&SelectedNode{
|
||||
LastNet: "192.168.1.0",
|
||||
}, &SelectedNode{
|
||||
LastNet: "192.168.2.0",
|
||||
},
|
||||
})
|
||||
|
||||
criteria := NodeFilters{}.
|
||||
WithExcludedIDs([]storj.NodeID{excluded}).
|
||||
WithAutoExcludeSubnets()
|
||||
|
||||
// due to node id criteria
|
||||
assert.False(t, criteria.MatchInclude(&SelectedNode{
|
||||
ID: excluded,
|
||||
LastNet: "192.168.0.1",
|
||||
LastNet: "192.168.1.0",
|
||||
}))
|
||||
|
||||
assert.False(t, criteria.MatchInclude(&SelectedNode{
|
||||
LastNet: "192.168.2.0",
|
||||
}))
|
||||
|
||||
// should be included as previous one excluded and
|
||||
// not stored for subnet exclusion
|
||||
assert.True(t, criteria.MatchInclude(&SelectedNode{
|
||||
ID: testrand.NodeID(),
|
||||
LastNet: "192.168.0.2",
|
||||
LastNet: "192.168.3.0",
|
||||
}))
|
||||
|
||||
}
|
||||
|
||||
func TestCriteria_Geofencing(t *testing.T) {
|
||||
@ -135,7 +118,6 @@ func BenchmarkNodeFilterFullTable(b *testing.B) {
|
||||
filters = append(filters, NodeFilterFunc(func(node *SelectedNode) bool {
|
||||
return true
|
||||
}))
|
||||
filters = filters.WithAutoExcludeSubnets()
|
||||
benchmarkFilter(b, filters)
|
||||
}
|
||||
|
||||
|
@ -248,12 +248,9 @@ func TestSelectFiltered(t *testing.T) {
|
||||
selector := nodeselection.SelectByID(nodes)
|
||||
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}), 3)
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}.WithAutoExcludeSubnets()), 2)
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}), 3)
|
||||
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}.WithExcludedIDs([]storj.NodeID{firstID, secondID})), 1)
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}.WithAutoExcludeSubnets()), 2)
|
||||
assert.Len(t, selector.Select(3, nodeselection.NodeFilters{}.WithExcludedIDs([]storj.NodeID{thirdID}).WithAutoExcludeSubnets()), 1)
|
||||
}
|
||||
|
||||
func TestSelectFilteredMulti(t *testing.T) {
|
||||
|
@ -12,6 +12,14 @@ import (
|
||||
"storj.io/common/storj"
|
||||
)
|
||||
|
||||
const (
|
||||
// AutoExcludeSubnet is placement annotation key to turn off subnet restrictions.
|
||||
AutoExcludeSubnet = "autoExcludeSubnet"
|
||||
|
||||
// AutoExcludeSubnetOFF is the value of AutoExcludeSubnet to disable subnet restrictions.
|
||||
AutoExcludeSubnetOFF = "off"
|
||||
)
|
||||
|
||||
// ErrNotEnoughNodes is when selecting nodes failed with the given parameters.
|
||||
var ErrNotEnoughNodes = errs.Class("not enough nodes")
|
||||
|
||||
@ -114,8 +122,13 @@ func (state *State) Select(ctx context.Context, request Request) (_ []*SelectedN
|
||||
|
||||
// Get all the remaining reputable nodes.
|
||||
reputableCount := totalCount - len(selected)
|
||||
selected = append(selected,
|
||||
reputableNodes.Select(reputableCount, request.NodeFilters)...)
|
||||
|
||||
filters := request.NodeFilters
|
||||
if GetAnnotation(filters, AutoExcludeSubnet) != AutoExcludeSubnetOFF {
|
||||
filters = append(append(NodeFilters{}, request.NodeFilters...), ExcludedNodeNetworks(selected))
|
||||
}
|
||||
|
||||
selected = append(selected, reputableNodes.Select(reputableCount, filters)...)
|
||||
|
||||
if len(selected) < totalCount {
|
||||
return selected, ErrNotEnoughNodes.New("requested from cache %d, found %d", totalCount, len(selected))
|
||||
|
@ -13,14 +13,6 @@ import (
|
||||
"storj.io/storj/satellite/nodeselection"
|
||||
)
|
||||
|
||||
const (
|
||||
// AutoExcludeSubnet is placement annotation key to turn off subnet restrictions.
|
||||
AutoExcludeSubnet = "autoExcludeSubnet"
|
||||
|
||||
// AutoExcludeSubnetOFF is the value of AutoExcludeSubnet to disable subnet restrictions.
|
||||
AutoExcludeSubnetOFF = "off"
|
||||
)
|
||||
|
||||
// UploadSelectionDB implements the database for upload selection cache.
|
||||
//
|
||||
// architecture: Database
|
||||
@ -105,7 +97,7 @@ func (cache *UploadSelectionCache) GetNodes(ctx context.Context, req FindStorage
|
||||
}
|
||||
|
||||
placementRules := cache.placementRules(req.Placement)
|
||||
useSubnetExclusion := nodeselection.GetAnnotation(placementRules, AutoExcludeSubnet) != AutoExcludeSubnetOFF
|
||||
useSubnetExclusion := nodeselection.GetAnnotation(placementRules, nodeselection.AutoExcludeSubnet) != nodeselection.AutoExcludeSubnetOFF
|
||||
|
||||
filters := nodeselection.NodeFilters{placementRules}
|
||||
if len(req.ExcludedIDs) > 0 {
|
||||
|
@ -215,7 +215,7 @@ func TestGetNodes(t *testing.T) {
|
||||
}
|
||||
placementRules := overlay.NewPlacementRules()
|
||||
placementRules.AddPlacementRule(storj.PlacementConstraint(5), nodeselection.NodeFilters{}.WithCountryFilter(location.NewSet(location.Germany)))
|
||||
placementRules.AddPlacementRule(storj.PlacementConstraint(6), nodeselection.WithAnnotation(nodeselection.NodeFilters{}.WithCountryFilter(location.NewSet(location.Germany)), overlay.AutoExcludeSubnet, overlay.AutoExcludeSubnetOFF))
|
||||
placementRules.AddPlacementRule(storj.PlacementConstraint(6), nodeselection.WithAnnotation(nodeselection.NodeFilters{}.WithCountryFilter(location.NewSet(location.Germany)), nodeselection.AutoExcludeSubnet, nodeselection.AutoExcludeSubnetOFF))
|
||||
|
||||
cache, err := overlay.NewUploadSelectionCache(zap.NewNop(),
|
||||
db.OverlayCache(),
|
||||
@ -526,7 +526,7 @@ func TestGetNodesDistinct(t *testing.T) {
|
||||
&mockDB,
|
||||
highStaleness,
|
||||
config,
|
||||
nodeselection.NodeFilters{}.WithAutoExcludeSubnets(),
|
||||
nodeselection.NodeFilters{},
|
||||
overlay.NewPlacementRules().CreateFilters,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
Loading…
Reference in New Issue
Block a user