From 7bc4ce25f2806b731999cec21d5c1e57e1c011c1 Mon Sep 17 00:00:00 2001 From: vyzo Date: Thu, 6 Jan 2022 18:55:45 +0200 Subject: [PATCH] named scopes for better errors to aid debugging resource allocation failures --- rcmgr.go | 14 ++++++------ scope.go | 62 ++++++++++++++++++++++++++++++++++----------------- scope_test.go | 32 +++++++++++++------------- 3 files changed, 64 insertions(+), 44 deletions(-) diff --git a/rcmgr.go b/rcmgr.go index 51467e8..ed03a97 100644 --- a/rcmgr.go +++ b/rcmgr.go @@ -257,20 +257,20 @@ func (r *resourceManager) gc() { func newSystemScope(limit Limit) *systemScope { return &systemScope{ - resourceScope: newResourceScope(limit, nil), + resourceScope: newResourceScope(limit, nil, "system"), } } func newTransientScope(limit Limit, system *systemScope) *transientScope { return &transientScope{ - resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}, "transient"), system: system, } } func newServiceScope(name string, limit Limit, system *systemScope) *serviceScope { return &serviceScope{ - resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}, fmt.Sprintf("service.%s", name)), name: name, system: system, } @@ -278,7 +278,7 @@ func newServiceScope(name string, limit Limit, system *systemScope) *serviceScop func newProtocolScope(proto protocol.ID, limit Limit, system *systemScope) *protocolScope { return &protocolScope{ - resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{system.resourceScope}, fmt.Sprintf("protocol.%s", proto)), proto: proto, system: system, } @@ -286,7 +286,7 @@ func newProtocolScope(proto protocol.ID, limit Limit, system *systemScope) *prot func newPeerScope(p peer.ID, limit Limit, rcmgr *resourceManager) *peerScope { return &peerScope{ - resourceScope: newResourceScope(limit, []*resourceScope{rcmgr.system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{rcmgr.system.resourceScope}, fmt.Sprintf("peer.%s", p)), peer: p, rcmgr: rcmgr, } @@ -294,7 +294,7 @@ func newPeerScope(p peer.ID, limit Limit, rcmgr *resourceManager) *peerScope { func newConnectionScope(dir network.Direction, usefd bool, limit Limit, rcmgr *resourceManager) *connectionScope { return &connectionScope{ - resourceScope: newResourceScope(limit, []*resourceScope{rcmgr.transient.resourceScope, rcmgr.system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{rcmgr.transient.resourceScope, rcmgr.system.resourceScope}, "connection"), dir: dir, usefd: usefd, rcmgr: rcmgr, @@ -303,7 +303,7 @@ func newConnectionScope(dir network.Direction, usefd bool, limit Limit, rcmgr *r func newStreamScope(dir network.Direction, limit Limit, peer *peerScope) *streamScope { return &streamScope{ - resourceScope: newResourceScope(limit, []*resourceScope{peer.resourceScope, peer.rcmgr.transient.resourceScope, peer.rcmgr.system.resourceScope}), + resourceScope: newResourceScope(limit, []*resourceScope{peer.resourceScope, peer.rcmgr.transient.resourceScope, peer.rcmgr.system.resourceScope}, "stream"), dir: dir, rcmgr: peer.rcmgr, peer: peer, diff --git a/scope.go b/scope.go index 8dbf2a6..9d838bb 100644 --- a/scope.go +++ b/scope.go @@ -35,18 +35,21 @@ type resourceScope struct { rc resources owner *resourceScope // set in transaction scopes, which define trees constraints []*resourceScope // set in DAG scopes, it's the linearized parent set + + name string // for debugging purposes } var _ network.ResourceScope = (*resourceScope)(nil) var _ network.TransactionalScope = (*resourceScope)(nil) -func newResourceScope(limit Limit, constraints []*resourceScope) *resourceScope { +func newResourceScope(limit Limit, constraints []*resourceScope, name string) *resourceScope { for _, cst := range constraints { cst.IncRef() } return &resourceScope{ rc: resources{limit: limit}, constraints: constraints, + name: name, } } @@ -54,6 +57,7 @@ func newTxnResourceScope(owner *resourceScope) *resourceScope { return &resourceScope{ rc: resources{limit: owner.rc.limit}, owner: owner, + name: fmt.Sprintf("%s.txn", owner.name), } } @@ -194,21 +198,25 @@ func (rc *resources) stat() network.ScopeStat { } // resourceScope implementation +func (s *resourceScope) wrapError(err error) error { + return fmt.Errorf("%s: %w", s.name, err) +} + func (s *resourceScope) ReserveMemory(size int, prio uint8) error { s.Lock() defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } if err := s.rc.reserveMemory(int64(size), prio); err != nil { - return err + return s.wrapError(err) } if err := s.reserveMemoryForConstraints(size, prio); err != nil { s.rc.releaseMemory(int64(size)) - return err + return s.wrapError(err) } return nil @@ -255,10 +263,14 @@ func (s *resourceScope) ReserveMemoryForChild(size int64, prio uint8) error { defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } - return s.rc.reserveMemory(size, prio) + if err := s.rc.reserveMemory(size, prio); err != nil { + return s.wrapError(err) + } + + return nil } func (s *resourceScope) ReleaseMemory(size int) { @@ -289,16 +301,16 @@ func (s *resourceScope) AddStream(dir network.Direction) error { defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } if err := s.rc.addStream(dir); err != nil { - return err + return s.wrapError(err) } if err := s.addStreamForConstraints(dir); err != nil { s.rc.removeStream(dir) - return err + return s.wrapError(err) } return nil @@ -332,10 +344,14 @@ func (s *resourceScope) AddStreamForChild(dir network.Direction) error { defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } - return s.rc.addStream(dir) + if err := s.rc.addStream(dir); err != nil { + return s.wrapError(err) + } + + return nil } func (s *resourceScope) RemoveStream(dir network.Direction) { @@ -377,16 +393,16 @@ func (s *resourceScope) AddConn(dir network.Direction, usefd bool) error { defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } if err := s.rc.addConn(dir, usefd); err != nil { - return err + return s.wrapError(err) } if err := s.addConnForConstraints(dir, usefd); err != nil { s.rc.removeConn(dir, usefd) - return err + return s.wrapError(err) } return nil @@ -420,10 +436,14 @@ func (s *resourceScope) AddConnForChild(dir network.Direction, usefd bool) error defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } - return s.rc.addConn(dir, usefd) + if err := s.rc.addConn(dir, usefd); err != nil { + return s.wrapError(err) + } + + return nil } func (s *resourceScope) RemoveConn(dir network.Direction, usefd bool) { @@ -464,22 +484,22 @@ func (s *resourceScope) ReserveForChild(st network.ScopeStat) error { defer s.Unlock() if s.done { - return network.ErrResourceScopeClosed + return s.wrapError(network.ErrResourceScopeClosed) } if err := s.rc.reserveMemory(st.Memory, network.ReservationPriorityAlways); err != nil { - return err + return s.wrapError(err) } if err := s.rc.addStreams(st.NumStreamsInbound, st.NumStreamsOutbound); err != nil { s.rc.releaseMemory(st.Memory) - return err + return s.wrapError(err) } if err := s.rc.addConns(st.NumConnsInbound, st.NumConnsOutbound, st.NumFD); err != nil { s.rc.releaseMemory(st.Memory) s.rc.removeStreams(st.NumStreamsInbound, st.NumStreamsOutbound) - return err + return s.wrapError(err) } return nil @@ -524,7 +544,7 @@ func (s *resourceScope) BeginTransaction() (network.TransactionalScope, error) { defer s.Unlock() if s.done { - return nil, network.ErrResourceScopeClosed + return nil, s.wrapError(network.ErrResourceScopeClosed) } s.refCnt++ diff --git a/scope_test.go b/scope_test.go index b5a711e..94766e3 100644 --- a/scope_test.go +++ b/scope_test.go @@ -243,7 +243,7 @@ func TestResourceScopeSimple(t *testing.T) { FD: 1, }, }, - nil, + nil, "test", ) s.IncRef() @@ -365,7 +365,7 @@ func TestResourceScopeTxnBasic(t *testing.T) { FD: 1, }, }, - nil, + nil, "test", ) txn, err := s.BeginTransaction() @@ -400,7 +400,7 @@ func TestResourceScopeTxnZombie(t *testing.T) { FD: 1, }, }, - nil, + nil, "test", ) txn1, err := s.BeginTransaction() @@ -442,7 +442,7 @@ func TestResourceScopeTxnTree(t *testing.T) { FD: 1, }, }, - nil, + nil, "test", ) txn1, err := s.BeginTransaction() @@ -551,7 +551,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 4, }, }, - nil, + nil, "test", ) s2 := newResourceScope( &StaticLimit{ @@ -564,7 +564,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 2, }, }, - []*resourceScope{s1}, + []*resourceScope{s1}, "test", ) s3 := newResourceScope( &StaticLimit{ @@ -577,7 +577,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 2, }, }, - []*resourceScope{s1}, + []*resourceScope{s1}, "test", ) s4 := newResourceScope( &StaticLimit{ @@ -590,7 +590,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 2, }, }, - []*resourceScope{s2, s3, s1}, + []*resourceScope{s2, s3, s1}, "test", ) s5 := newResourceScope( &StaticLimit{ @@ -603,7 +603,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 2, }, }, - []*resourceScope{s2, s1}, + []*resourceScope{s2, s1}, "test", ) s6 := newResourceScope( &StaticLimit{ @@ -616,7 +616,7 @@ func TestResourceScopeDAG(t *testing.T) { FD: 2, }, }, - []*resourceScope{s3, s1}, + []*resourceScope{s3, s1}, "test", ) if err := s4.ReserveMemory(1024, network.ReservationPriorityAlways); err != nil { @@ -1056,37 +1056,37 @@ func TestResourceScopeDAGTxn(t *testing.T) { &StaticLimit{ Memory: 8192, }, - nil, + nil, "test", ) s2 := newResourceScope( &StaticLimit{ Memory: 4096 + 2048, }, - []*resourceScope{s1}, + []*resourceScope{s1}, "test", ) s3 := newResourceScope( &StaticLimit{ Memory: 4096 + 2048, }, - []*resourceScope{s1}, + []*resourceScope{s1}, "test", ) s4 := newResourceScope( &StaticLimit{ Memory: 4096 + 1024, }, - []*resourceScope{s2, s3, s1}, + []*resourceScope{s2, s3, s1}, "test", ) s5 := newResourceScope( &StaticLimit{ Memory: 4096 + 1024, }, - []*resourceScope{s2, s1}, + []*resourceScope{s2, s1}, "test", ) s6 := newResourceScope( &StaticLimit{ Memory: 4096 + 1024, }, - []*resourceScope{s3, s1}, + []*resourceScope{s3, s1}, "test", ) txn4, err := s4.BeginTransaction()