Skip to content

Commit 51d7f95

Browse files
akerouantonakerouanton
committed
libnet: remove struct endpointCnt
endpointCnt is a refcounter used to track how many endpoints use a network, and how many networks references a config-only network. It's stored separately from the network. This is only used to determine if a network can be removed. This commit removes the `endpointCnt` struct and all its references. The refcounter is replaced by two lookups in the newly introduced `networks` and `endpoints` caches added to the `Controller`. Signed-off-by: Albin Kerouanton <[email protected]>
1 parent d377cd3 commit 51d7f95

10 files changed

Lines changed: 127 additions & 288 deletions

File tree

‎internal/maputil/maputil.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
2+
//go:build go1.22
3+
4+
package maputil
5+
6+
func FilterValues[K comparable, V any](in map[K]V, fn func(V) bool) []V {
7+
var out []V
8+
for _, v := range in {
9+
if fn(v) {
10+
out = append(out, v)
11+
}
12+
}
13+
return out
14+
}

‎libnetwork/controller.go‎

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -554,8 +554,6 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ...
554554
var (
555555
caps driverapi.Capability
556556
err error
557-
558-
skipCfgEpCount bool
559557
)
560558

561559
// Reset network types, force local scope and skip allocation and
@@ -590,13 +588,6 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ...
590588
if err := configNetwork.applyConfigurationTo(nw); err != nil {
591589
return nil, types.InternalErrorf("Failed to apply configuration: %v", err)
592590
}
593-
defer func() {
594-
if retErr == nil && !skipCfgEpCount {
595-
if err := configNetwork.getEpCnt().IncEndpointCnt(); err != nil {
596-
log.G(context.TODO()).Warnf("Failed to update reference count for configuration network %q on creation of network %q: %v", configNetwork.Name(), nw.name, err)
597-
}
598-
}
599-
}()
600591
}
601592

602593
// At this point the network scope is still unknown if not set by user
@@ -662,11 +653,7 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ...
662653
//
663654
// To cut a long story short: if this broke anything, you know who to blame :)
664655
if err := c.addNetwork(nw); err != nil {
665-
if _, ok := err.(types.MaskableError); ok { //nolint:gosimple
666-
// This error can be ignored and set this boolean
667-
// value to skip a refcount increment for configOnly networks
668-
skipCfgEpCount = true
669-
} else {
656+
if _, ok := err.(types.MaskableError); !ok { //nolint:gosimple
670657
return nil, err
671658
}
672659
}
@@ -694,22 +681,6 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ...
694681
}
695682

696683
addToStore:
697-
// First store the endpoint count, then the network. To avoid to
698-
// end up with a datastore containing a network and not an epCnt,
699-
// in case of an ungraceful shutdown during this function call.
700-
epCnt := &endpointCnt{n: nw}
701-
if err := c.updateToStore(context.TODO(), epCnt); err != nil {
702-
return nil, err
703-
}
704-
defer func() {
705-
if retErr != nil {
706-
if err := c.deleteFromStore(epCnt); err != nil {
707-
log.G(context.TODO()).Warnf("could not rollback from store, epCnt %v on failure (%v): %v", epCnt, retErr, err)
708-
}
709-
}
710-
}()
711-
712-
nw.epCnt = epCnt
713684
if err := c.storeNetwork(context.TODO(), nw); err != nil {
714685
return nil, err
715686
}

‎libnetwork/endpoint.go‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,10 +1047,6 @@ func (ep *Endpoint) Delete(ctx context.Context, force bool) error {
10471047

10481048
ep.releaseAddress()
10491049

1050-
if err := n.getEpCnt().DecEndpointCnt(); err != nil {
1051-
log.G(ctx).Warnf("failed to decrement endpoint count for ep %s: %v", ep.ID(), err)
1052-
}
1053-
10541050
return nil
10551051
}
10561052

@@ -1399,20 +1395,6 @@ func (c *Controller) cleanupLocalEndpoints() error {
13991395
log.G(context.TODO()).Warnf("Could not delete local endpoint %s during endpoint cleanup: %v", ep.name, err)
14001396
}
14011397
}
1402-
1403-
epl, err = n.getEndpointsFromStore()
1404-
if err != nil {
1405-
log.G(context.TODO()).Warnf("Could not get list of endpoints in network %s for count update: %v", n.name, err)
1406-
continue
1407-
}
1408-
1409-
epCnt := n.getEpCnt().EndpointCnt()
1410-
if epCnt != uint64(len(epl)) {
1411-
log.G(context.TODO()).Infof("Fixing inconsistent endpoint_cnt for network %s. Expected=%d, Actual=%d", n.name, len(epl), epCnt)
1412-
if err := n.getEpCnt().setCnt(uint64(len(epl))); err != nil {
1413-
log.G(context.TODO()).WithField("network", n.name).WithError(err).Warn("Error while fixing inconsistent endpoint_cnt for network")
1414-
}
1415-
}
14161398
}
14171399

14181400
return nil

‎libnetwork/endpoint_cnt.go‎

Lines changed: 0 additions & 173 deletions
This file was deleted.

‎libnetwork/endpoint_store.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1+
// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
2+
//go:build go1.22
3+
14
package libnetwork
25

3-
import "context"
6+
import (
7+
"context"
8+
9+
"github.com/docker/docker/internal/maputil"
10+
)
411

512
// storeEndpoint inserts or updates the endpoint in the store and the in-memory
613
// cache maintained by the Controller.
@@ -39,3 +46,21 @@ func (c *Controller) cacheEndpoint(ep *Endpoint) {
3946
defer c.endpointsMu.Unlock()
4047
c.endpoints[ep.id] = ep
4148
}
49+
50+
// findEndpoints looks for all endpoints matching the filter from the in-memory
51+
// cache of endpoints maintained by the Controller.
52+
//
53+
// This method is thread-safe, but do not use it unless you're sure your code
54+
// uses the returned endpoints in thread-safe way (see the comment on
55+
// Controller.endpoints).
56+
func (c *Controller) findEndpoints(filter func(ep *Endpoint) bool) []*Endpoint {
57+
c.endpointsMu.Lock()
58+
defer c.endpointsMu.Unlock()
59+
return maputil.FilterValues(c.endpoints, filter)
60+
}
61+
62+
func filterEndpointByNetworkId(expected string) func(ep *Endpoint) bool {
63+
return func(ep *Endpoint) bool {
64+
return ep.network != nil && ep.network.id == expected
65+
}
66+
}

‎libnetwork/endpoint_store_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,21 @@ func TestEndpointStore(t *testing.T) {
2525
err = c.storeEndpoint(context.Background(), ep2)
2626
assert.NilError(t, err)
2727

28+
// Check that we can find both endpoints
29+
found := c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
30+
assert.Equal(t, len(found), 2)
31+
assert.Equal(t, found[0], ep1)
32+
assert.Equal(t, found[1], ep2)
33+
2834
// Delete the first endpoint
2935
err = c.deleteStoredEndpoint(ep1)
3036
assert.NilError(t, err)
3137

38+
// Check that we can only find the second endpoint
39+
found = c.findEndpoints(filterEndpointByNetworkId("testNetwork"))
40+
assert.Equal(t, len(found), 1)
41+
assert.Equal(t, found[0], ep2)
42+
3243
// Store the second endpoint again
3344
err = c.storeEndpoint(context.Background(), ep2)
3445
assert.NilError(t, err)

0 commit comments

Comments
 (0)