Network Resource Manager interface #229

vyzo · 2021-12-16T12:48:18Z

See libp2p/go-libp2p#1260 for the proposal/design discussion.

network/rcmgr.go

Stebalien · 2021-12-16T17:45:32Z

network/rcmgr.go

+// TransactionalScope is a mixin interface for transactional scopes.
+type TransactionalScope interface {
+	// Done ends the transaction scope and releases associated resources.
+	Done()


Do we have any examples of how we think we might want to use these interfaces? We should start with some toy applications before we spend too much time on these APIs.

The most basic hooks will be inside the transport and muxer implementations.
We need to cater to both them, at the bowels of the stack, and higher level users.

Can we add testable examples to the godocs? https://go.dev/blog/examples

marten-seemann

Initial pass.

network/rcmgr.go

…tters

marten-seemann

Quick review today, more to follow tomorrow.

network/rcmgr.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

network/rcmgr.go

The stream is unnegotiated state until the actual protocol has been determined.

…method

1. Introduce transactions in all scopes 2. Limit the view of stream/connection scope for users, to avoid the Done footgun 3. Move OpenStream to the resource manager

…cmgr

network/rcmgr.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

…mory status so that we don't have to undo reservations if there is too much pressure.

It needs to attach the protocol to the protocol scope, which may fail.

network/rcmgr.go

Co-authored-by: raulk <raul@protocol.ai>

network/rcmgr.go

aschmahmann · 2022-01-11T20:00:49Z

network/rcmgr.go

+	ViewTransient(func(ResourceScope) error) error
+
+	// ViewService retrieves a service-specific scope.
+	ViewService(string, func(ServiceScope) error) error


Does service deal with transient connections too? How does hole punching figure into this (e.g. when secondary connections associated with the service are made)?

No, the service does not deal with conns at all, it only deals with streams.
Conns are really shared resources, so there isn't much of a point to have them owned by a service.

network/rcmgr.go

transport/transport.go

aschmahmann · 2022-01-13T16:12:46Z

network/rcmgr.go

+	Stat() ScopeStat
+
+	// BeginTransaction creates a new transactional scope rooted at this scope
+	BeginTransaction() (TransactionalScope, error)


Under what kinds of situations are we expecting an error here? Is this just room for growth in case anything comes up, or do we have some in mind?

We currently error if the scope has been closed. But it's good to have room for growth too.

network/rcmgr.go

aschmahmann · 2022-01-13T16:29:55Z

network/rcmgr.go

+	NumStreamsInbound  int
+	NumStreamsOutbound int
+	NumConnsInbound    int
+	NumConnsOutbound   int


How (if at all) are we planning to deal with these in the context of relayed or DCUtR connections?

what do you mean here? Relayed connections are just a virtual connection on top of some other connection and don't use fds.

We do count them however in stat.

Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>

github-actions · 2022-01-14T10:17:09Z

gocompat says:

Branch 'master' set up to track remote branch 'master' from 'origin'.
"github.com/libp2p/go-libp2p-core/mux".Multiplexer DeclarationTypeChanged
"github.com/libp2p/go-libp2p-core/mux".MuxedConn DeclarationTypeChanged
"github.com/libp2p/go-libp2p-core/mux".MuxedStream DeclarationTypeChanged
"github.com/libp2p/go-libp2p-core/mux".NoopHandler TopLevelDeclarationDeleted
"github.com/libp2p/go-libp2p-core/network".Network InterfaceChanged
"github.com/libp2p/go-libp2p-core/network".Stream InterfaceChanged
"github.com/libp2p/go-libp2p-core/network".Conn InterfaceChanged
"github.com/libp2p/go-libp2p-core/transport".CapableConn InterfaceChanged
"github.com/libp2p/go-libp2p-core/transport".TransportNetwork InterfaceChanged
"github.com/libp2p/go-libp2p-core/transport".Upgrader InterfaceChanged

vyzo added 3 commits December 16, 2021 14:42

add resource manager interfaces

cfce3f5

add scope accessors to streams and conns

044fbea

add ResourceManager accessor to the Network interface

193ee25

vyzo requested review from Stebalien, raulk and marten-seemann December 16, 2021 12:48

vyzo marked this pull request as draft December 16, 2021 12:48

allow initially unattached streams.

2afdeee

Stebalien reviewed Dec 16, 2021

View reviewed changes

marten-seemann reviewed Dec 16, 2021

View reviewed changes

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

vyzo added 4 commits December 21, 2021 14:33

introduce service scopes, canonicalize ownership interface through se…

46cbcfe

…tters

make system scope explicit

d4cfc93

make memory stat an int64

586f1d0

make the system scope a generic resource scope, introduce the DMZ

b3f9e1d

marten-seemann reviewed Dec 21, 2021

View reviewed changes

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Show resolved Hide resolved

vyzo and others added 3 commits December 21, 2021 17:02

fix typo

35ec4fd

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

fix typo

081233f

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

Merge branch 'master' into feat/rcmgr

5767cbb

marten-seemann reviewed Dec 22, 2021

View reviewed changes

network/rcmgr.go Outdated Show resolved Hide resolved

vyzo added 2 commits December 22, 2021 11:17

rename DMZ to transient scope, remove OpenConnection from PeerScope

aeb9233

remove ncopy param from GrowBuffer

d997bbb

marten-seemann mentioned this pull request Dec 22, 2021

use the Resource Manager libp2p/go-libp2p-transport-upgrader#99

Merged

vyzo added 5 commits December 22, 2021 13:18

remove protocols from OpenStream

8f759d3

The stream is unnegotiated state until the actual protocol has been determined.

document nil receiver contract, fix protocol scope protocol accessor …

dcef937

…method

remove nil receiver contract requirement

c52807c

flesh out stat struct

76a25c9

turn resource manager scope accessors into viewers

25d4931

vyzo force-pushed the feat/rcmgr branch from 56c9220 to 25d4931 Compare December 23, 2021 19:03

vyzo added 2 commits December 24, 2021 11:03

interface refiniments

deb1c16

1. Introduce transactions in all scopes 2. Limit the view of stream/connection scope for users, to avoid the Done footgun 3. Move OpenStream to the resource manager

add Buffer interface

1f5a414

vyzo and others added 5 commits January 4, 2022 11:38

Merge branch 'master' of github.com:libp2p/go-libp2p-core into feat/r…

59c3e97

…cmgr

add the ConnManagementScope to Upgrader.Upgrade

f4f1520

fix argument name

d2ff788

godocs for ResourceManager

d1a82f1

introduce MemoryStatus indicator in ReserveMemory

541324d

marten-seemann reviewed Jan 5, 2022

View reviewed changes

network/rcmgr.go Outdated Show resolved Hide resolved

vyzo and others added 5 commits January 5, 2022 17:18

use uint8 for MemoryStatus

cafe95a

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

rework reservation interface to pass priority instead of returning me…

84d55d3

…mory status so that we don't have to undo reservations if there is too much pressure.

improve comment

d9136f0

fix typo

2192774

export the NullScope

005b84b

vyzo marked this pull request as ready for review January 8, 2022 16:10

vyzo and others added 3 commits January 9, 2022 15:12

Stream.SetProtocol can return an error

9d473a8

It needs to attach the protocol to the protocol scope, which may fail.

merge the mux package into network

0c1046c

pass the PeerScope to Multiplexer.NetConn

0391e67

marten-seemann force-pushed the feat/rcmgr branch from 3042c2a to 0391e67 Compare January 13, 2022 11:19

raulk reviewed Jan 13, 2022

View reviewed changes

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

network/rcmgr.go Outdated Show resolved Hide resolved

vyzo and others added 3 commits January 13, 2022 14:58

Update network/rcmgr.go

d3d9ac7

Co-authored-by: raulk <raul@protocol.ai>

Update network/rcmgr.go

140d0ed

Co-authored-by: raulk <raul@protocol.ai>

Update network/rcmgr.go

7a1c47e

Co-authored-by: raulk <raul@protocol.ai>

aschmahmann reviewed Jan 13, 2022

View reviewed changes

vyzo and others added 6 commits January 14, 2022 11:57

Update network/rcmgr.go

e2bac5c

Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>

remove reference to deprecated mux.MuxedConn

4bfde8e

rename transaction to span

68e492f

indicate bytes in ReserveMemory

953320f

break ResourceManager View methods into Viewer interface.

06aa6f9

add experimental interface warning

6b8d8bf

marten-seemann approved these changes Jan 17, 2022

View reviewed changes

marten-seemann merged commit e4c76cf into master Jan 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Network Resource Manager interface #229

Network Resource Manager interface #229

vyzo commented Dec 16, 2021

Stebalien Dec 16, 2021

vyzo Dec 16, 2021

raulk Dec 30, 2021

marten-seemann left a comment

marten-seemann left a comment

aschmahmann Jan 11, 2022

vyzo Jan 14, 2022

aschmahmann Jan 13, 2022

vyzo Jan 14, 2022

aschmahmann Jan 13, 2022

vyzo Jan 14, 2022

vyzo Jan 14, 2022

github-actions bot commented Jan 14, 2022

Network Resource Manager interface #229

Network Resource Manager interface #229

Conversation

vyzo commented Dec 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marten-seemann left a comment

Choose a reason for hiding this comment

marten-seemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 14, 2022