Skip to content

Commit

Permalink
Use values instead of pointers for PartialLimitConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
MarcoPolo committed Jan 30, 2023
1 parent c68e24c commit 2f95f28
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 41 deletions.
14 changes: 4 additions & 10 deletions p2p/host/resource-manager/limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ func limitValFromInt64(i int64, defaultVal int64) LimitVal64 {
}

// ToResourceLimits converts the BaseLimit to a ResourceLimits
func (l BaseLimit) ToResourceLimits() *ResourceLimits {
return &ResourceLimits{
func (l BaseLimit) ToResourceLimits() ResourceLimits {
return ResourceLimits{
Streams: valueOrBlockAll(l.Streams),
StreamsInbound: valueOrBlockAll(l.StreamsInbound),
StreamsOutbound: valueOrBlockAll(l.StreamsOutbound),
Expand All @@ -146,8 +146,8 @@ func (l BaseLimit) ToResourceLimits() *ResourceLimits {
}
}

func (l BaseLimit) ToResourceLimitsWithDefault(defaultLimit BaseLimit) *ResourceLimits {
out := ResourceLimits{
func (l BaseLimit) ToResourceLimitsWithDefault(defaultLimit BaseLimit) ResourceLimits {
return ResourceLimits{
Streams: limitValFromInt(l.Streams, defaultLimit.Streams),
StreamsInbound: limitValFromInt(l.StreamsInbound, defaultLimit.StreamsInbound),
StreamsOutbound: limitValFromInt(l.StreamsOutbound, defaultLimit.StreamsOutbound),
Expand All @@ -157,12 +157,6 @@ func (l BaseLimit) ToResourceLimitsWithDefault(defaultLimit BaseLimit) *Resource
FD: limitValFromInt(l.FD, defaultLimit.FD),
Memory: limitValFromInt64(l.Memory, defaultLimit.Memory),
}

if out.IsDefault() {
return nil
}

return &out
}

// Apply overwrites all zero-valued limits with the values of l2
Expand Down
4 changes: 2 additions & 2 deletions p2p/host/resource-manager/limit_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestLimitConfigRoundTrip(t *testing.T) {
func TestReadmeLimitConfigSerialization(t *testing.T) {
noisyNeighbor, _ := peer.Decode("QmVvtzcZgCkMnSFf2dnrBPXrWuNFWNM9J3MpZQCvWPuVZf")
cfg := PartialLimitConfig{
System: &ResourceLimits{
System: ResourceLimits{
// Allow unlimited outbound streams
StreamsOutbound: Unlimited,
},
Expand All @@ -146,5 +146,5 @@ func TestReadmeLimitConfigSerialization(t *testing.T) {
}
jsonBytes, err := json.Marshal(&cfg)
require.NoError(t, err)
require.Equal(t, `{"System":{"StreamsOutbound":"unlimited"},"Peer":{"QmVvtzcZgCkMnSFf2dnrBPXrWuNFWNM9J3MpZQCvWPuVZf":{"StreamsInbound":"blockAll","StreamsOutbound":"unlimited","ConnsInbound":"blockAll"}}}`, string(jsonBytes))
require.Equal(t, `{"Peer":{"QmVvtzcZgCkMnSFf2dnrBPXrWuNFWNM9J3MpZQCvWPuVZf":{"StreamsInbound":"blockAll","StreamsOutbound":"unlimited","ConnsInbound":"blockAll"}},"System":{"StreamsOutbound":"unlimited"}}`, string(jsonBytes))
}
76 changes: 55 additions & 21 deletions p2p/host/resource-manager/limit_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,15 @@ func (l *ResourceLimits) IsDefault() bool {
return false
}

// Apply overwrites all default limits with the values of l2
func (l *ResourceLimits) Apply(l2 *ResourceLimits) {
if l2 == nil {
return
func (l *ResourceLimits) ToMaybeNilPtr() *ResourceLimits {
if l.IsDefault() {
return nil
}
return l
}

// Apply overwrites all default limits with the values of l2
func (l *ResourceLimits) Apply(l2 ResourceLimits) {
if l.Streams == DefaultLimit {
l.Streams = l2.Streams
}
Expand Down Expand Up @@ -315,32 +319,32 @@ func (l *ResourceLimits) Build(defaults BaseLimit) BaseLimit {
}

type PartialLimitConfig struct {
System *ResourceLimits `json:",omitempty"`
Transient *ResourceLimits `json:",omitempty"`
System ResourceLimits `json:",omitempty"`
Transient ResourceLimits `json:",omitempty"`

// Limits that are applied to resources with an allowlisted multiaddr.
// These will only be used if the normal System & Transient limits are
// reached.
AllowlistedSystem *ResourceLimits `json:",omitempty"`
AllowlistedTransient *ResourceLimits `json:",omitempty"`
AllowlistedSystem ResourceLimits `json:",omitempty"`
AllowlistedTransient ResourceLimits `json:",omitempty"`

ServiceDefault *ResourceLimits `json:",omitempty"`
ServiceDefault ResourceLimits `json:",omitempty"`
Service map[string]ResourceLimits `json:",omitempty"`

ServicePeerDefault *ResourceLimits `json:",omitempty"`
ServicePeerDefault ResourceLimits `json:",omitempty"`
ServicePeer map[string]ResourceLimits `json:",omitempty"`

ProtocolDefault *ResourceLimits `json:",omitempty"`
ProtocolDefault ResourceLimits `json:",omitempty"`
Protocol map[protocol.ID]ResourceLimits `json:",omitempty"`

ProtocolPeerDefault *ResourceLimits `json:",omitempty"`
ProtocolPeerDefault ResourceLimits `json:",omitempty"`
ProtocolPeer map[protocol.ID]ResourceLimits `json:",omitempty"`

PeerDefault *ResourceLimits `json:",omitempty"`
PeerDefault ResourceLimits `json:",omitempty"`
Peer map[peer.ID]ResourceLimits `json:",omitempty"`

Conn *ResourceLimits `json:",omitempty"`
Stream *ResourceLimits `json:",omitempty"`
Conn ResourceLimits `json:",omitempty"`
Stream ResourceLimits `json:",omitempty"`
}

func resourceLimitsMapFromBaseLimitMapWithDefaults[K comparable](m map[K]BaseLimit, defaultLimits map[K]BaseLimit, fallbackDefault BaseLimit) map[K]ResourceLimits {
Expand All @@ -355,9 +359,7 @@ func resourceLimitsMapFromBaseLimitMapWithDefaults[K comparable](m map[K]BaseLim
def = defaultForKey
}
rl := v.ToResourceLimitsWithDefault(def)
if rl != nil {
out[k] = *rl
}
out[k] = rl
}
return out
}
Expand All @@ -372,18 +374,50 @@ func (cfg *PartialLimitConfig) MarshalJSON() ([]byte, error) {
type Alias PartialLimitConfig
return json.Marshal(&struct {
*Alias
// String so we can have the properly marshalled peer id
Peer map[string]ResourceLimits `json:",omitempty"`

// The rest of the fields as pointers so that we omit empty values in the serialized result
System *ResourceLimits `json:",omitempty"`
Transient *ResourceLimits `json:",omitempty"`
AllowlistedSystem *ResourceLimits `json:",omitempty"`
AllowlistedTransient *ResourceLimits `json:",omitempty"`

ServiceDefault *ResourceLimits `json:",omitempty"`

ServicePeerDefault *ResourceLimits `json:",omitempty"`

ProtocolDefault *ResourceLimits `json:",omitempty"`

ProtocolPeerDefault *ResourceLimits `json:",omitempty"`

PeerDefault *ResourceLimits `json:",omitempty"`

Conn *ResourceLimits `json:",omitempty"`
Stream *ResourceLimits `json:",omitempty"`
}{
Alias: (*Alias)(cfg),
Peer: encodedPeerMap,

System: cfg.System.ToMaybeNilPtr(),
Transient: cfg.Transient.ToMaybeNilPtr(),
AllowlistedSystem: cfg.AllowlistedSystem.ToMaybeNilPtr(),
AllowlistedTransient: cfg.AllowlistedTransient.ToMaybeNilPtr(),
ServiceDefault: cfg.ServiceDefault.ToMaybeNilPtr(),
ServicePeerDefault: cfg.ServicePeerDefault.ToMaybeNilPtr(),
ProtocolDefault: cfg.ProtocolDefault.ToMaybeNilPtr(),
ProtocolPeerDefault: cfg.ProtocolPeerDefault.ToMaybeNilPtr(),
PeerDefault: cfg.PeerDefault.ToMaybeNilPtr(),
Conn: cfg.Conn.ToMaybeNilPtr(),
Stream: cfg.Stream.ToMaybeNilPtr(),
})
}

func applyResourceLimitsMap[K comparable](this *map[K]ResourceLimits, other map[K]ResourceLimits, fallbackDefault *ResourceLimits) {
func applyResourceLimitsMap[K comparable](this *map[K]ResourceLimits, other map[K]ResourceLimits, fallbackDefault ResourceLimits) {
for k, l := range *this {
r := fallbackDefault
if l2, ok := other[k]; ok {
r = &l2
r = l2
}
l.Apply(r)
(*this)[k] = l
Expand Down Expand Up @@ -536,7 +570,7 @@ func resourceLimitsMapFromBaseLimitMap[K comparable](baseLimitMap map[K]BaseLimi

out := make(map[K]ResourceLimits)
for k, l := range baseLimitMap {
out[k] = *l.ToResourceLimits()
out[k] = l.ToResourceLimits()
}

return out
Expand Down
4 changes: 2 additions & 2 deletions p2p/host/resource-manager/limit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,12 @@ func TestJSONRoundTripInt64(t *testing.T) {

func TestRoundTripFromConcreteAndBack(t *testing.T) {
l := PartialLimitConfig{
System: &ResourceLimits{
System: ResourceLimits{
Conns: 1234,
Memory: 54321,
},

ServiceDefault: &ResourceLimits{
ServiceDefault: ResourceLimits{
Conns: 2,
},

Expand Down
12 changes: 6 additions & 6 deletions p2p/test/resource-manager/rcmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ func TestResourceManagerConnInbound(t *testing.T) {
// this test checks that we can not exceed the inbound conn limit at system level
// we specify: 1 conn per peer, 3 conns total, and we try to create 4 conns
cfg := rcmgr.PartialLimitConfig{
System: &rcmgr.ResourceLimits{
System: rcmgr.ResourceLimits{
ConnsInbound: 3,
ConnsOutbound: 1024,
Conns: 1024,
StreamsOutbound: rcmgr.Unlimited,
},
PeerDefault: &rcmgr.ResourceLimits{
PeerDefault: rcmgr.ResourceLimits{
ConnsInbound: 1,
ConnsOutbound: 1,
Conns: 1,
Expand Down Expand Up @@ -90,12 +90,12 @@ func TestResourceManagerConnOutbound(t *testing.T) {
// this test checks that we can not exceed the inbound conn limit at system level
// we specify: 1 conn per peer, 3 conns total, and we try to create 4 conns
cfg := rcmgr.PartialLimitConfig{
System: &rcmgr.ResourceLimits{
System: rcmgr.ResourceLimits{
ConnsInbound: 1024,
ConnsOutbound: 3,
Conns: 1024,
},
PeerDefault: &rcmgr.ResourceLimits{
PeerDefault: rcmgr.ResourceLimits{
ConnsInbound: 1,
ConnsOutbound: 1,
Conns: 1,
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestResourceManagerServiceInbound(t *testing.T) {
// this test checks that we can not exceed the inbound stream limit at service level
// we specify: 3 streams for the service, and we try to create 4 streams
cfg := rcmgr.PartialLimitConfig{
ServiceDefault: &rcmgr.ResourceLimits{
ServiceDefault: rcmgr.ResourceLimits{
StreamsInbound: 3,
StreamsOutbound: 1024,
Streams: 1024,
Expand Down Expand Up @@ -305,7 +305,7 @@ func TestReadmeExample(t *testing.T) {

// Tweak certain settings
cfg := rcmgr.PartialLimitConfig{
System: &rcmgr.ResourceLimits{
System: rcmgr.ResourceLimits{
// Allow unlimited outbound streams
StreamsOutbound: rcmgr.Unlimited,
},
Expand Down

0 comments on commit 2f95f28

Please sign in to comment.