Skip to content

Commit 75690a1

Browse files
committed
Reuse released versiond child ports
Replace the monotonic child port counter with a bounded pool that scans from BasePort and reuses ports after child exit. Reserve versiond's own listen port so long-lived supervisors cannot eventually assign :8080 to a child, and normalize invalid BasePort values back to the default. Update manager tests for port reuse, reserved-port skipping, and out-of-range BasePort handling. Document the bounded child-port pool in the rolling update design.
1 parent e056edc commit 75690a1

3 files changed

Lines changed: 72 additions & 10 deletions

File tree

devshard/docs/rolling-update.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,9 @@ func (m *Manager) assignPort(name string) int {
281281
```
282282
→ add a swap-aware allocation (e.g. `assignSwapPort(name)`) that returns a new
283283
port even when `name` already has one, and a `releasePort` on drain completion.
284+
The implementation uses a bounded child-port pool starting at `BasePort`,
285+
reuses ports after child exit, and reserves versiond's own listen port so a
286+
long-lived supervisor does not eventually allocate it to a child.
284287

285288
#### c) Readiness gate instead of TCP-accept
286289

versioned/internal/process/manager.go

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"os/exec"
1515
"path/filepath"
1616
"sort"
17+
"strconv"
1718
"strings"
1819
"sync"
1920
"sync/atomic"
@@ -35,6 +36,8 @@ const (
3536
statusStopped = "stopped"
3637

3738
childLoopbackHost = "127.0.0.1"
39+
invalidChildPort = -1
40+
maxChildPort = 65535
3841
devshardMetaDBFile = "_meta.db"
3942
defaultDevshardShutdownGrace = 10 * time.Minute
4043
installedVersionRetain = 3
@@ -64,7 +67,7 @@ type Manager struct {
6467
draining map[string][]*child
6568
downloading map[string]struct{}
6669
allocatedPorts map[int]struct{}
67-
nextPort int
70+
reservedPorts map[int]struct{}
6871
mu sync.Mutex
6972
routes atomic.Value // map[string]string
7073
}
@@ -77,14 +80,14 @@ func NewManager(cfg config.Config) *Manager {
7780
draining: make(map[string][]*child),
7881
downloading: make(map[string]struct{}),
7982
allocatedPorts: make(map[int]struct{}),
80-
nextPort: cfg.BasePort,
83+
reservedPorts: reservedChildPorts(),
8184
}
8285
m.routes.Store(map[string]string{})
8386
return m
8487
}
8588

8689
func normalizeConfig(cfg config.Config) config.Config {
87-
if cfg.BasePort == 0 {
90+
if cfg.BasePort <= 0 || cfg.BasePort > maxChildPort {
8891
cfg.BasePort = 5000
8992
}
9093
if cfg.ReadyPath == "" {
@@ -114,21 +117,49 @@ func normalizeConfig(cfg config.Config) config.Config {
114117
// assignPort returns a currently-free child port.
115118
// Must be called with m.mu held.
116119
func (m *Manager) assignPort() int {
117-
for {
118-
port := m.nextPort
119-
m.nextPort++
120+
for port := m.cfg.BasePort; port <= maxChildPort; port++ {
120121
if _, used := m.allocatedPorts[port]; used {
121122
continue
122123
}
124+
if _, reserved := m.reservedPorts[port]; reserved {
125+
continue
126+
}
123127
m.allocatedPorts[port] = struct{}{}
124128
return port
125129
}
130+
slog.Error("no child ports available", "base_port", m.cfg.BasePort, "max_port", maxChildPort)
131+
return invalidChildPort
132+
}
133+
134+
func reservedChildPorts() map[int]struct{} {
135+
ports := make(map[int]struct{})
136+
if port, ok := parseListenPort(config.ListenAddr()); ok {
137+
ports[port] = struct{}{}
138+
}
139+
return ports
140+
}
141+
142+
func parseListenPort(addr string) (int, bool) {
143+
_, portStr, err := net.SplitHostPort(addr)
144+
if err != nil {
145+
if !strings.HasPrefix(addr, ":") {
146+
slog.Warn("cannot parse versiond listen address for child port reservation", "addr", addr, "error", err)
147+
return 0, false
148+
}
149+
portStr = strings.TrimPrefix(addr, ":")
150+
}
151+
port, err := strconv.Atoi(portStr)
152+
if err != nil || port <= 0 || port > maxChildPort {
153+
slog.Warn("cannot parse versiond listen port for child port reservation", "addr", addr, "port", portStr, "error", err)
154+
return 0, false
155+
}
156+
return port, true
126157
}
127158

128159
// releasePort releases a child port after the child process exits.
129160
// Must be called with m.mu held.
130161
func (m *Manager) releasePort(port int) {
131-
if port != 0 {
162+
if port > 0 {
132163
delete(m.allocatedPorts, port)
133164
}
134165
}

versioned/internal/process/manager_test.go

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ func TestHashFile_Missing(t *testing.T) {
470470
}
471471
}
472472

473-
func TestAssignPort_AllocatesOverlapPortsAndReleases(t *testing.T) {
473+
func TestAssignPort_ReusesReleasedPorts(t *testing.T) {
474474
cfg := config.Config{BasePort: 5000}
475475
m := NewManager(cfg)
476476

@@ -487,8 +487,36 @@ func TestAssignPort_AllocatesOverlapPortsAndReleases(t *testing.T) {
487487
if p2 != 5001 {
488488
t.Errorf("second port = %d, want 5001", p2)
489489
}
490-
if p3 != 5002 {
491-
t.Errorf("released port should not be immediately reused; got %d, want 5002", p3)
490+
if p3 != 5000 {
491+
t.Errorf("released port should be reused; got %d, want 5000", p3)
492+
}
493+
}
494+
495+
func TestAssignPort_SkipsVersiondListenPort(t *testing.T) {
496+
m := NewManager(config.Config{BasePort: 8079})
497+
498+
m.mu.Lock()
499+
p1 := m.assignPort()
500+
p2 := m.assignPort()
501+
m.mu.Unlock()
502+
503+
if p1 != 8079 {
504+
t.Errorf("first port = %d, want 8079", p1)
505+
}
506+
if p2 != 8081 {
507+
t.Errorf("second port = %d, want 8081", p2)
508+
}
509+
}
510+
511+
func TestAssignPort_NormalizesOutOfRangeBasePort(t *testing.T) {
512+
m := NewManager(config.Config{BasePort: 70000})
513+
514+
m.mu.Lock()
515+
port := m.assignPort()
516+
m.mu.Unlock()
517+
518+
if port != 5000 {
519+
t.Errorf("port = %d, want 5000", port)
492520
}
493521
}
494522

0 commit comments

Comments
 (0)