diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index a4ecb2be..f580bed2 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -56,6 +56,14 @@ const ( sandboxCreationTimeout = 2 * time.Minute // storeCleanupTimeout is the maximum duration allowed to clean up a store placeholder. storeCleanupTimeout = 30 * time.Second + // claimReadyPollInitialInterval is the delay before the second read of a + // SandboxClaim while waiting for it to bind a ready Sandbox. A claim served + // from a warm pool is usually bound within a few hundred milliseconds, so + // the first retries come quickly. + claimReadyPollInitialInterval = 100 * time.Millisecond + // claimReadyPollMaxInterval caps the delay between reads; the interval + // doubles from claimReadyPollInitialInterval until it reaches this value. + claimReadyPollMaxInterval = 1 * time.Second ) // isContextError reports whether err is a context cancellation or deadline error. @@ -331,8 +339,9 @@ func (s *Server) waitForClaimSandboxReady(ctx context.Context, dynamicClient dyn func (s *Server) waitForClaimSandboxReadyWithTimeout(ctx context.Context, dynamicClient dynamic.Interface, sandboxClaim *extensionsv1alpha1.SandboxClaim, timeout time.Duration) (*sandboxv1alpha1.Sandbox, error) { waitCtx, cancel := context.WithTimeoutCause(ctx, timeout, errSandboxCreationTimeout) defer cancel() - ticker := time.NewTicker(1 * time.Second) - defer ticker.Stop() + interval := claimReadyPollInitialInterval + timer := time.NewTimer(interval) + defer timer.Stop() for { claim, err := getSandboxClaim(waitCtx, dynamicClient, sandboxClaim.Namespace, sandboxClaim.Name) @@ -375,7 +384,9 @@ func (s *Server) waitForClaimSandboxReadyWithTimeout(ctx context.Context, dynami klog.Warningf("sandbox claim %s/%s wait canceled: %v", sandboxClaim.Namespace, sandboxClaim.Name, waitErr) } return nil, waitErr - case <-ticker.C: + case <-timer.C: + interval = min(interval*2, claimReadyPollMaxInterval) + timer.Reset(interval) } } } diff --git a/pkg/workloadmanager/handlers_bench_test.go b/pkg/workloadmanager/handlers_bench_test.go new file mode 100644 index 00000000..25a72ec4 --- /dev/null +++ b/pkg/workloadmanager/handlers_bench_test.go @@ -0,0 +1,89 @@ +/* +Copyright The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package workloadmanager + +import ( + "context" + "sync/atomic" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + dynamicfake "k8s.io/client-go/dynamic/fake" + k8stesting "k8s.io/client-go/testing" + extensionsv1alpha1 "sigs.k8s.io/agent-sandbox/extensions/api/v1alpha1" +) + +// BenchmarkWaitForClaimSandboxReady measures how long waitForClaimSandboxReady +// takes to notice that a SandboxClaim has been bound to a ready Sandbox. The +// fake API server binds the claim claimBindDelay after the wait starts, as the +// agent-sandbox controller does when it adopts a sandbox from a warm pool. The +// reported ns/op is therefore the wait latency, and gets/op the number of +// claim and sandbox reads issued to the API server. +func BenchmarkWaitForClaimSandboxReady(b *testing.B) { + for _, claimBindDelay := range []time.Duration{20 * time.Millisecond, 300 * time.Millisecond} { + b.Run("bind"+claimBindDelay.String(), func(b *testing.B) { + claim := &extensionsv1alpha1.SandboxClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "ci-claim", Namespace: "ns-1"}, + } + unboundObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(claim) + if err != nil { + b.Fatal(err) + } + bound := claim.DeepCopy() + bound.Status.SandboxStatus.Name = "sandbox-1" + boundObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(bound) + if err != nil { + b.Fatal(err) + } + sandboxObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(readySandbox()) + if err != nil { + b.Fatal(err) + } + + var gets atomic.Int64 + var start atomic.Int64 + dynamicClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + dynamicClient.PrependReactor("get", "*", func(action k8stesting.Action) (bool, runtime.Object, error) { + gets.Add(1) + switch action.GetResource() { + case SandboxClaimGVR: + if time.Since(time.Unix(0, start.Load())) < claimBindDelay { + return true, &unstructured.Unstructured{Object: unboundObj}, nil + } + return true, &unstructured.Unstructured{Object: boundObj}, nil + case SandboxGVR: + return true, &unstructured.Unstructured{Object: sandboxObj}, nil + } + return false, nil, nil + }) + + s := &Server{} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + start.Store(time.Now().UnixNano()) + if _, err := s.waitForClaimSandboxReady(context.Background(), dynamicClient, claim); err != nil { + b.Fatal(err) + } + } + b.ReportMetric(float64(gets.Load())/float64(b.N), "gets/op") + }) + } +} diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index 823ccb34..0b2f8089 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -639,6 +639,42 @@ func TestWaitForClaimSandboxReadyCancelsInFlightGet(t *testing.T) { } } +func TestWaitForClaimSandboxReadyNoticesQuickBindingWithoutWaitingASecond(t *testing.T) { + claim := &extensionsv1alpha1.SandboxClaim{ + ObjectMeta: metav1.ObjectMeta{Name: "ci-claim", Namespace: "ns-1"}, + } + unboundObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(claim) + require.NoError(t, err) + bound := claim.DeepCopy() + bound.Status.SandboxStatus.Name = "sandbox-1" + boundObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(bound) + require.NoError(t, err) + sandboxObject, err := runtime.DefaultUnstructuredConverter.ToUnstructured(readySandbox()) + require.NoError(t, err) + + start := time.Now() + dynamicClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + dynamicClient.PrependReactor("get", "*", func(action k8stesting.Action) (bool, runtime.Object, error) { + switch action.GetResource() { + case SandboxClaimGVR: + if time.Since(start) < 20*time.Millisecond { + return true, &unstructured.Unstructured{Object: unboundObject}, nil + } + return true, &unstructured.Unstructured{Object: boundObject}, nil + case SandboxGVR: + return true, &unstructured.Unstructured{Object: sandboxObject}, nil + default: + return false, nil, nil + } + }) + + createdSandbox, err := (&Server{}).waitForClaimSandboxReady(context.Background(), dynamicClient, claim) + + require.NoError(t, err) + require.NotNil(t, createdSandbox) + require.Less(t, time.Since(start), 700*time.Millisecond, "claim bound after 20ms should not wait for a 1s tick") +} + func TestWaitForClaimSandboxReadyRejectsLateReadySandbox(t *testing.T) { claim := &extensionsv1alpha1.SandboxClaim{ ObjectMeta: metav1.ObjectMeta{Name: "ci-claim", Namespace: "ns-1"},