Skip to content

Commit b9e2b42

Browse files
committed
Bound SSH filesystem PID probe wait
1 parent 9441a8c commit b9e2b42

3 files changed

Lines changed: 125 additions & 3 deletions

File tree

‎experimental/ssh/internal/fuse/export_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ var ParseRegistration = parseRegistration
66

77
const RefreshInterval = refreshInterval
88

9+
const PIDProbeTimeout = pidProbeTimeout
10+
911
func ConfigureTestClient(c *Client, httpClient *http.Client, hosts []string, readPID func() (int, error)) {
1012
c.http = httpClient
1113
c.hosts = hosts

‎experimental/ssh/internal/fuse/keepalive.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import (
1111
"github.com/databricks/cli/libs/log"
1212
)
1313

14-
const refreshInterval = time.Minute
14+
const (
15+
refreshInterval = time.Minute
16+
pidProbeTimeout = 2 * time.Second
17+
)
1518

1619
// TokenFunc returns the current credential, including any refresh performed by the SDK.
1720
type TokenFunc func(context.Context) (string, error)
@@ -36,15 +39,32 @@ func refreshUntilDone(ctx context.Context, client *Client, token TokenFunc, user
3639
ticker := time.NewTicker(refreshInterval)
3740
defer ticker.Stop()
3841
defer client.http.CloseIdleConnections()
42+
var pendingProbe chan bool
3943
for {
4044
select {
4145
case <-ctx.Done():
4246
return
4347
case <-ticker.C:
44-
pid, err := client.readPID()
48+
if pendingProbe == nil {
49+
pendingProbe = make(chan bool, 1)
50+
go func(result chan<- bool) {
51+
pid, err := client.readPID()
52+
result <- err != nil || pid != client.registration.PID
53+
}(pendingProbe)
54+
}
4555
// Re-register WSFS only after credential rotation or loss of the registered ancestor.
4656
// Unchanged WSFS registrations unnecessarily invalidate the daemon's caches.
47-
force := err != nil || pid != client.registration.PID
57+
force := true
58+
timer := time.NewTimer(pidProbeTimeout)
59+
select {
60+
case <-ctx.Done():
61+
timer.Stop()
62+
return
63+
case force = <-pendingProbe:
64+
pendingProbe = nil
65+
case <-timer.C:
66+
}
67+
timer.Stop()
4868
if err := refresh(ctx, client, token, userID, notebookDir, force); err != nil {
4969
log.Warnf(ctx, "Failed to refresh SSH filesystem credentials; retrying in %v: %v", refreshInterval, err)
5070
}

‎experimental/ssh/internal/fuse/keepalive_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,103 @@ func TestKeepRegisteredRecoversVolumesRegistrationLoss(t *testing.T) {
175175
}
176176
})
177177
}
178+
179+
type keepaliveTransport struct {
180+
http.RoundTripper
181+
closed chan struct{}
182+
}
183+
184+
func (transport *keepaliveTransport) CloseIdleConnections() {
185+
close(transport.closed)
186+
}
187+
188+
func TestKeepRegisteredBlockedPIDProbe(t *testing.T) {
189+
for _, scenario := range []string{"cancel", "recover"} {
190+
t.Run(scenario, func(t *testing.T) {
191+
synctest.Test(t, func(t *testing.T) {
192+
ctx, cancel := context.WithCancel(t.Context())
193+
defer cancel()
194+
client, err := fuse.NewClient(registration)
195+
require.NoError(t, err)
196+
var mu sync.Mutex
197+
requests := map[string]int{}
198+
transport := &keepaliveTransport{
199+
closed: make(chan struct{}),
200+
RoundTripper: roundTripFunc(func(request *http.Request) (*http.Response, error) {
201+
mu.Lock()
202+
defer mu.Unlock()
203+
requests[request.URL.Port()]++
204+
return &http.Response{StatusCode: http.StatusOK, Body: http.NoBody}, nil
205+
}),
206+
}
207+
httpClient := fuse.HTTPClient(client)
208+
httpClient.Transport = transport
209+
releaseProbe := make(chan struct{}, 1)
210+
defer close(releaseProbe)
211+
probeCalls := 0
212+
fuse.ConfigureTestClient(client, httpClient, testHosts, func() (int, error) {
213+
mu.Lock()
214+
probeCalls++
215+
firstProbe := probeCalls == 1
216+
mu.Unlock()
217+
if firstProbe {
218+
<-releaseProbe
219+
}
220+
return registration.PID, nil
221+
})
222+
require.NoError(t, fuse.KeepRegistered(ctx, client, func(context.Context) (string, error) {
223+
return "fixed-bootstrap-token", nil
224+
}, "12345", testNotebookDir))
225+
synctest.Wait()
226+
start := time.Now()
227+
for tick := range 3 {
228+
time.Sleep(time.Until(start.Add(time.Duration(tick+1) * fuse.RefreshInterval)))
229+
synctest.Wait()
230+
mu.Lock()
231+
assert.Equal(t, tick+1, requests["1015"], "wait for the probe before refreshing")
232+
mu.Unlock()
233+
time.Sleep(fuse.PIDProbeTimeout)
234+
synctest.Wait()
235+
mu.Lock()
236+
assert.Equal(t, tick+2, requests["1015"], "keep refreshing volumes while WSFS is blocked")
237+
assert.Equal(t, tick+2, requests["1021"], "force WSFS registration after a probe timeout")
238+
assert.Equal(t, 1, probeCalls, "reuse the outstanding probe across refreshes")
239+
mu.Unlock()
240+
}
241+
242+
if scenario == "recover" {
243+
releaseProbe <- struct{}{}
244+
synctest.Wait()
245+
time.Sleep(2 * fuse.RefreshInterval)
246+
synctest.Wait()
247+
mu.Lock()
248+
assert.Equal(t, 6, requests["1015"])
249+
assert.Equal(t, 4, requests["1021"], "stop forcing WSFS registration after probe recovery")
250+
assert.Equal(t, 2, probeCalls, "resume probing after the outstanding read completes")
251+
mu.Unlock()
252+
} else {
253+
time.Sleep(fuse.RefreshInterval - fuse.PIDProbeTimeout)
254+
synctest.Wait()
255+
}
256+
257+
cancel()
258+
synctest.Wait()
259+
select {
260+
case <-transport.closed:
261+
default:
262+
assert.Fail(t, "refresh loop must exit on cancellation even with a blocked probe")
263+
}
264+
mu.Lock()
265+
callsAtCancel := probeCalls
266+
requestsAtCancel := requests["1015"]
267+
mu.Unlock()
268+
time.Sleep(2 * fuse.RefreshInterval)
269+
synctest.Wait()
270+
mu.Lock()
271+
assert.Equal(t, callsAtCancel, probeCalls)
272+
assert.Equal(t, requestsAtCancel, requests["1015"])
273+
mu.Unlock()
274+
})
275+
})
276+
}
277+
}

0 commit comments

Comments
 (0)