feat(sdk/go): add client-side sandbox pool with in-memory state store#1173
feat(sdk/go): add client-side sandbox pool with in-memory state store#1173zhaoyaohua726 wants to merge 6 commits into
Conversation
Implement OSEP-0005 client-side pool for the Go SDK, enabling millisecond-level sandbox acquisition through pre-warmed idle instances. Features: - Configurable pool size with automatic reconciliation - Acquire with DirectCreate/FailFast policies - Health checking and stale instance recycling - Exponential backoff on consecutive warmup failures - InMemoryPoolStateStore for single-process deployments - Graceful shutdown with drain timeout - Resize, ReleaseAllIdle, Snapshot operations Closes opensandbox-group#1166 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zhaoyaohua726
requested review from
Pangjiping,
hittyt,
jwx0925 and
ninan-nn
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be235640bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Image: spec.Image, | ||
| Entrypoint: spec.Entrypoint, | ||
| ResourceLimits: spec.ResourceLimits, | ||
| TimeoutSeconds: &timeout, |
There was a problem hiding this comment.
When CreationSpec.Timeout is left at its zero value, warmupOne creates pooled sandboxes with DefaultTimeoutSeconds (600s) but records the idle membership until IdleTimeout (24h by default) later in the function. After about 10 minutes the server can terminate every warmed sandbox while CountIdle still sees a full pool for another 23h50m, so reconciliation stops replenishing and prewarmed acquisition degrades to stale IDs or direct creates. Use the pool idle timeout, or renew to it just before PutIdle, for warmup sandboxes.
Useful? React with 👍 / 👎.
| select { | ||
| case <-p.doneCh: | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
When Shutdown(context.Background()) is called before Start has launched the reconcile goroutine, or after Start failed validation, no goroutine will ever close doneCh, so this wait blocks forever; a common defer pool.Shutdown(...) cleanup path can therefore hang the process. Track whether the reconciler was started or skip the doneCh wait from the CREATED state.
Useful? React with 👍 / 👎.
| // Skip expired entries | ||
| if time.Now().After(entry.ExpiresAt) { | ||
| return "", false, nil |
There was a problem hiding this comment.
If the FIFO head has expired but a later entry is still valid, this returns (empty, false) after removing only the expired head. That makes Acquire(FailFast) report an empty pool and ReleaseAllIdle stop early even though borrowable sandboxes remain; lazy expiry should continue scanning until it finds a non-expired entry or the queue is exhausted.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| // Try to get an idle sandbox | ||
| sandboxID, ok, err := p.config.StateStore.TryTakeIdle(ctx, p.config.PoolName) |
There was a problem hiding this comment.
PoolConfig exposes AcquireMinRemainingTTL and defaults it to skip entries near expiry, but Acquire calls TryTakeIdle without passing that threshold, and the store API only filters fully expired entries. With an idle sandbox that has less than the configured remaining TTL, e.g. 30s left with the default 60s threshold, the pool can hand it out without renewal unless the caller supplied WithSandboxTimeout, defeating the option and returning near-expiry resources.
Useful? React with 👍 / 👎.
| case FailFast: | ||
| return nil, &PoolEmptyError{PoolName: p.config.PoolName} |
There was a problem hiding this comment.
When an idle candidate is found but connectAndValidate fails, the code discards the original connection/health/renew error and reaches this FailFast branch as if the pool were simply empty. In stale or unhealthy-id scenarios, callers cannot distinguish a depleted pool from a broken pool entry even though PoolAcquireFailedError exists for exactly this case; return that failure with the sandbox ID and underlying error instead of PoolEmptyError.
Useful? React with 👍 / 👎.
|
|
||
| // Put into idle set | ||
| expiresAt := time.Now().Add(p.config.IdleTimeout) | ||
| if err := p.config.StateStore.PutIdle(ctx, p.config.PoolName, sb.ID(), expiresAt); err != nil { |
There was a problem hiding this comment.
warmupOne writes the newly created sandbox to the shared store without verifying that this pool instance still owns the primary lock. If sandbox creation or the preparer takes longer than PrimaryLockTTL, another owner can acquire leadership and start replenishing while the old owner still executes this PutIdle, so two owners can publish warmups for the same deficit and overfill the pool; renew/check the lock immediately before each publish and discard created sandboxes if ownership was lost.
Useful? React with 👍 / 👎.
| atomic.AddInt32(&p.inFlight, 1) | ||
| defer atomic.AddInt32(&p.inFlight, -1) |
There was a problem hiding this comment.
After the initial RUNNING check, a concurrent Shutdown can switch the pool to DRAINING and observe zero in-flight operations before this increment happens, then finish cleanup while this Acquire still proceeds to take or directly create a sandbox. Recheck the lifecycle state after incrementing inFlight so shutdown actually stops accepting new acquire requests and cannot return a new sandbox after the pool has stopped.
Useful? React with 👍 / 👎.
| p.mu.Unlock() | ||
|
|
||
| // Signal reconciler to stop | ||
| close(p.stopCh) |
There was a problem hiding this comment.
If two goroutines call Shutdown while the first one has moved the state to DRAINING but has not yet reached STOPPED, the second call does not return early and will also execute close(p.stopCh), which panics on a closed channel. This can happen with overlapping signal handlers or duplicated cleanup paths; guard the close with a once/closed flag or treat DRAINING as an already-started shutdown.
Useful? React with 👍 / 👎.
| select { | ||
| case <-p.doneCh: | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
When callers pass a timeout/deadline context to Shutdown and it expires while a reconcile tick is still warming sandboxes, this branch continues as if the reconciler had exited: it releases current idle entries, marks the pool STOPPED, and returns nil. The still-running tick can then PutIdle newly created sandboxes after cleanup, leaving live idle sandboxes behind a stopped pool; return the context error or otherwise wait/cancel the reconcile work before final cleanup.
Useful? React with 👍 / 👎.
| if maxIdle <= 0 { | ||
| return &InvalidArgumentError{Field: "maxIdle", Message: "must be positive"} | ||
| } |
There was a problem hiding this comment.
Rejecting maxIdle == 0 means callers cannot drain or temporarily disable replenishment without shutting down the pool; even ReleaseAllIdle can be immediately refilled because the target remains positive. Other pool implementations and the OSEP lifecycle allow resize(0) as the supported way to converge the idle reserve to zero, so this check prevents an important operational control path.
Useful? React with 👍 / 👎.
Fixes all issues raised by the automated reviewer: P1 - Align warmup sandbox TTL with idle membership: Use IdleTimeout as the sandbox server-side TTL so pool membership and server expiration stay in sync. P2 - Avoid blocking shutdown before reconciler starts: Track whether the reconciler goroutine launched; skip doneCh wait if Start was never called. P2 - Continue past expired idle entries in TryTakeIdle: Scan past expired entries instead of returning empty on first hit. P2 - Return PoolAcquireFailedError under FailFast: When an idle sandbox fails connectAndValidate, return the actual error (not just PoolEmptyError) so callers can distinguish. P2 - Recheck lock before publishing warmups: RenewLock before PutIdle to prevent overfilling if lock expired during sandbox creation. P2 - Prevent acquire racing past shutdown: Increment inFlight before checking state, ensuring Shutdown sees the operation. P2 - Make shutdown idempotent: Use sync.Once to guard close(stopCh), preventing double-close panic. P2 - Cancel reconcile before final cleanup: Use a dedicated context for reconcile work, cancelled on shutdown. P2 - Allow resize to zero: Accept maxIdle=0 to disable replenishment without shutdown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add end-to-end integration tests covering: - Acquire from pre-warmed idle pool (warmup + acquire + run command) - FailFast policy returns PoolEmptyError when pool is empty - DirectCreate fallback creates sandbox on-demand - Resize and ReleaseAllIdle operations - Graceful shutdown cleans up idle sandboxes Tests are gated behind RUN_POOL_E2E=true env var since they require a running OpenSandbox server. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e17470f9d8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if t == 0 { | ||
| t = DefaultTimeoutSeconds | ||
| } |
There was a problem hiding this comment.
When CreationSpec.Timeout is omitted, an empty pool falls through here and returns a directly-created sandbox with the SDK's 600s default TTL, while warmed pool sandboxes use IdleTimeout (24h by default). The same Acquire call therefore yields a sandbox that can be reaped after 10 minutes only on a pool miss; use the pool idle timeout, or renew to it, for the direct-create fallback unless the caller supplied WithSandboxTimeout.
Useful? React with 👍 / 👎.
| t = DefaultTimeoutSeconds | ||
| } | ||
|
|
||
| return CreateSandbox(ctx, p.config.ConnectionConfig, SandboxCreateOptions{ |
There was a problem hiding this comment.
When a pool miss uses direct create, this CreateSandbox call only passes the custom HealthCheck; it omits ReadyTimeout and HealthCheckInterval, so a user who raised AcquireReadyTimeout for slow readiness still gets the default 30s timeout on misses even though idle hits use the configured timeout in connectAndValidate. Pass the acquire timeout and interval here as well.
Useful? React with 👍 / 👎.
| } | ||
| p.mu.Lock() | ||
| defer p.mu.Unlock() | ||
| p.config.MaxIdle = maxIdle |
There was a problem hiding this comment.
When multiple pool instances share a state store, only the current primary's local p.config.MaxIdle is read during reconciliation. A follower calling Resize(ctx, 0) updates only its own memory here, so the leader continues replenishing at the old target and the logical pool cannot be drained or resized unless every process is updated; persist the target in shared state or otherwise make the leader observe resize requests.
Useful? React with 👍 / 👎.
| sandboxID, ok, err := p.config.StateStore.TryTakeIdle(ctx, p.config.PoolName) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("pool %q: state store error: %w", p.config.PoolName, err) | ||
| } |
There was a problem hiding this comment.
With the default DirectCreate policy, a transient distributed-state-store error here prevents acquisition even if the lifecycle API is healthy and a cold sandbox could be created. That makes the pool less available than direct SDK usage during Redis/etcd/store outages; only FailFast should fail immediately, while DirectCreate should treat the failed idle take as a miss or retry before falling back.
Useful? React with 👍 / 👎.
| sb, err := CreateSandbox(ctx, p.config.ConnectionConfig, SandboxCreateOptions{ | ||
| Image: spec.Image, | ||
| Entrypoint: spec.Entrypoint, | ||
| ResourceLimits: spec.ResourceLimits, | ||
| TimeoutSeconds: &timeout, |
There was a problem hiding this comment.
This warmup create path does not pass the pool's custom HealthCheck or readiness timing options, so pools that rely on a service-specific health check only validate that condition during Acquire. In that configuration the supposedly prewarmed sandbox can still spend up to AcquireReadyTimeout becoming ready on the foreground request path, defeating the latency benefit and potentially causing avoidable stale-id fallback.
Useful? React with 👍 / 👎.
| failureCount, lastError, backoffActive := p.recon.snapshot() | ||
|
|
||
| return PoolSnapshot{ | ||
| State: state, |
There was a problem hiding this comment.
After repeated warmup failures, reconcileState marks backoff/degraded internally, but Snapshot always reports only the lifecycle state captured here, so callers still see PoolStateRunning even while replenishment is backed off. Operators relying on Snapshot().State cannot detect degraded pools; map the reconciler health into the snapshot state or add explicit HEALTHY/DEGRADED states.
Useful? React with 👍 / 👎.
| case <-ctx.Done(): | ||
| reconCancel() | ||
| return |
There was a problem hiding this comment.
If Start is called with a deadline or request context, cancellation exits the reconciler here but leaves p.state as PoolStateRunning; subsequent Snapshot and Acquire calls make the pool look healthy even though no background replenishment will ever run again. Either decouple the worker lifetime from the caller context or transition the pool out of RUNNING when this branch fires.
Useful? React with 👍 / 👎.
|
|
||
| // PoolCreationSpec is the template for creating sandboxes in the pool. | ||
| type PoolCreationSpec struct { | ||
| Image string |
There was a problem hiding this comment.
PoolCreationSpec only stores the image URI, so pool warmups and direct-create fallback cannot pass ImageAuth even though regular CreateSandbox supports private registry credentials. A pool configured for an image that requires authentication will fail every create while the same image works through the non-pool Go SDK; carry the auth fields through the pool spec and create calls.
Useful? React with 👍 / 👎.
|
|
||
| done: | ||
| // Kill all remaining idle sandboxes | ||
| p.ReleaseAllIdle(context.Background()) |
There was a problem hiding this comment.
If Start fails validation because StateStore is nil, a normal cleanup path such as defer pool.Shutdown(ctx) still reaches this unconditional idle cleanup and then dereferences the nil store inside ReleaseAllIdle/ReleaseLock, panicking instead of being a no-op. Guard shutdown cleanup for pools that never successfully started or have no valid store.
Useful? React with 👍 / 👎.
|
|
||
| done: | ||
| // Kill all remaining idle sandboxes | ||
| p.ReleaseAllIdle(context.Background()) |
There was a problem hiding this comment.
When ReleaseAllIdle fails against a distributed store during shutdown, this call discards the returned error and the method still marks the pool STOPPED, so callers cannot know that idle IDs may remain in the store and live sandboxes may still be consuming quota. Propagate the cleanup error or avoid transitioning to STOPPED until the remaining idle entries are accounted for.
Useful? React with 👍 / 👎.
The project CI (real-e2e.yml) already starts a full server + Docker environment for Go e2e tests. Pool tests only need the default image (python:3.11-slim), no special setup required. Remove the env var gate so tests run automatically in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 641e4c6ca2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // PoolCreationSpec is the template for creating sandboxes in the pool. | ||
| type PoolCreationSpec struct { | ||
| Image string | ||
| Timeout int | ||
| Entrypoint []string | ||
| ResourceLimits ResourceLimits | ||
| Env map[string]string | ||
| Metadata map[string]string | ||
| NetworkPolicy *NetworkPolicy | ||
| Volumes []Volume | ||
| Extensions map[string]string | ||
| } |
There was a problem hiding this comment.
When callers need secured gateway endpoints, regular Go sandbox creation supports SandboxCreateOptions.SecureAccess, and the Python/Kotlin pool specs expose the same setting, but this pool template has no SecureAccess field to forward in warmup or direct-create paths. A sandbox acquired from the new Go pool is therefore always created with secureAccess=false, so migrating the same workload to pooled acquisition silently loses the endpoint protection and there is no API-level way to request it.
Useful? React with 👍 / 👎.
| if timeout > 0 { | ||
| secs := int(timeout.Seconds()) | ||
| t = secs |
There was a problem hiding this comment.
When a pool miss uses DirectCreate with WithSandboxTimeout, this code turns that duration into the initial create TTL, but CreateSandbox waits for the sandbox to become ready before returning. The server-side TTL therefore starts before readiness and health checks, unlike the idle path that renews after connect, so slow starts can return a sandbox with far less than the requested TTL or one that is already near expiry; create with the pool TTL and renew to sandboxTimeout after readiness instead.
Useful? React with 👍 / 👎.
| Volumes []Volume | ||
| Extensions map[string]string | ||
| } |
There was a problem hiding this comment.
For workloads that enable Credential Vault's transparent proxy, regular Go sandbox creation forwards SandboxCreateOptions.CredentialProxy, and the Kotlin pool template carries the same setting, but the new Go pool template ends without any CredentialProxy field to pass to warmup or direct-create calls. Pooled sandboxes are therefore created without the credential proxy even when the equivalent non-pooled CreateSandbox call works, so code using in-sandbox credential injection cannot migrate to this pool API.
Useful? React with 👍 / 👎.
| // PoolCreationSpec is the template for creating sandboxes in the pool. | ||
| type PoolCreationSpec struct { | ||
| Image string | ||
| Timeout int | ||
| Entrypoint []string | ||
| ResourceLimits ResourceLimits | ||
| Env map[string]string | ||
| Metadata map[string]string | ||
| NetworkPolicy *NetworkPolicy | ||
| Volumes []Volume | ||
| Extensions map[string]string | ||
| } |
There was a problem hiding this comment.
For OS/architecture-specific workloads, regular Go sandbox creation accepts SandboxCreateOptions.Platform, and the Python/Kotlin pool creation specs carry the same platform constraint, but this new Go pool template has no Platform field to forward. A pooled Windows or arm64 workload can therefore be created with the server's default platform instead of the caller's required platform, making the same image/configuration work via non-pooled CreateSandbox but fail or start on the wrong target when acquired through this pool.
Useful? React with 👍 / 👎.
Fixes all 10 issues from the second automated review: - Use idle timeout for direct-create fallback TTL (consistent with warmed) - Honor acquire readiness settings (ReadyTimeout, HealthCheckInterval, HealthCheck) on both direct-create and warmup paths - Add ImageAuth to PoolCreationSpec for private registry support - Fall back to direct-create on transient state store errors (DirectCreate policy) - Pass HealthCheck to warmup CreateSandbox for pre-validation - Expose PoolStateDegraded in Snapshot when backoff is active - Transition to STOPPED when reconciler context is cancelled - Guard Shutdown against nil StateStore (safe after failed Start) - Return ReleaseAllIdle errors from Shutdown to surface cleanup failures Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b97685ed0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if p.state != PoolStateCreated { | ||
| p.mu.Unlock() | ||
| return &PoolNotRunningError{PoolName: p.config.PoolName, State: p.state} |
There was a problem hiding this comment.
When a Go pool has already gone through Shutdown, this state check rejects the next Start because the state is PoolStateStopped, whereas the existing Python and Kotlin pool implementations recreate their scheduler/resources from STOPPED. In restart-after-maintenance flows that are covered by the other SDKs, Go callers must throw away the pool object and rebuild it instead of reusing the same configured pool; reinitialize the stop/done channels and reconciler when starting from STOPPED rather than treating it as an invalid state.
Useful? React with 👍 / 👎.
| Image string | ||
| Timeout int | ||
| Entrypoint []string | ||
| ResourceLimits ResourceLimits | ||
| Env map[string]string |
There was a problem hiding this comment.
PoolCreationSpec exposes ResourceLimits but has no ResourceRequests field, and the warmup/direct-create paths therefore never set SandboxCreateOptions.ResourceRequests. For Kubernetes workloads that rely on lower requests than limits for scheduling or Burstable QoS, migrating the same Go CreateSandbox options to a pool silently falls back to using limits as requests, which can change QoS and make sandboxes unnecessarily unschedulable.
Useful? React with 👍 / 👎.
| case <-ctx.Done(): | ||
| reconCancel() | ||
| return |
There was a problem hiding this comment.
When the context passed to Start is canceled after the pool has warmed idle sandboxes, this branch exits the reconciler and the defer marks the pool STOPPED; a later Shutdown(context.Background()) then treats the pool as already stopped and skips ReleaseAllIdle, leaving live idle sandboxes in the store until their server TTL expires. Either decouple the worker lifetime from the caller's setup context or run the same idle cleanup path before transitioning to STOPPED on context cancellation.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| // Put into idle set with TTL aligned to sandbox server-side expiration | ||
| expiresAt := time.Now().Add(time.Duration(timeout) * time.Second) |
There was a problem hiding this comment.
When readiness checks or SandboxPreparer take non-trivial time, the server-side TTL has already been counting since CreateSandbox, but this stamps the store expiry from the later publish time, so CountIdle and acquire TTL filtering can consider a sandbox borrowable after it is already near expiry or gone. Fresh evidence in this revision is that warmup now stamps expiresAt after the preparer without renewing the sandbox to the same timeout immediately before PutIdle.
Useful? React with 👍 / 👎.
| func (s *InMemoryPoolStateStore) CountIdle(_ context.Context, poolName string) (int, error) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| return len(s.idle[poolName]), nil |
There was a problem hiding this comment.
When no reconcile sweep has run since entries expired (for example with a short IdleTimeout or after the reconciler stopped), CountIdle still reports those expired entries even though TryTakeIdle will skip them. That makes Snapshot().IdleCount and any readiness loop built on it report a warm pool while Acquire(FailFast) can immediately see the pool as empty; reap or filter expired entries before returning the count.
Useful? React with 👍 / 👎.
| r.lastError = errMsg | ||
| if r.failureCount >= r.threshold { | ||
| r.backoffAttempts++ | ||
| backoff := time.Duration(1<<uint(r.backoffAttempts-1)) * 30 * time.Second |
There was a problem hiding this comment.
During a long outage, backoffAttempts keeps increasing each time the capped 24h backoff expires and another warmup fails, but this shift/multiply overflows time.Duration before the maxBackoff comparison can cap it. After enough degraded cycles the computed backoff becomes negative or zero, so the pool stops honoring the intended 24h cap and resumes create attempts every reconcile interval against an unhealthy backend.
Useful? React with 👍 / 👎.
Fixes all 7 P2 issues from the third automated review: - Allow stopped pools to restart (reinitialize channels/state from STOPPED) - Add ResourceRequests to PoolCreationSpec for Burstable QoS support - Clean up idle entries when reconciler context is cancelled - Renew server-side TTL after preparer before stamping idle expiry - Exclude expired entries from CountIdle to avoid misleading snapshots - Clamp backoff bit-shift to prevent time.Duration overflow - Restartability parity with Python/Kotlin SDKs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1c43f33a0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if p.state == PoolStateRunning { | ||
| p.mu.Unlock() | ||
| return &PoolNotRunningError{PoolName: p.config.PoolName, State: p.state} | ||
| } |
There was a problem hiding this comment.
When Start races with an in-progress Shutdown, the state is PoolStateDraining, but this guard only rejects RUNNING, so the call flips the pool back to RUNNING and launches another reconciler using the same already-closed stopCh/doneCh. In that lifecycle overlap, the old and new reconcile loops can both close doneCh, causing a close of closed channel panic or cleanup to run while shutdown is still in progress; wait for STOPPED or reject DRAINING before restarting.
Useful? React with 👍 / 👎.
| lock := s.locks[poolName] | ||
| if lock == nil || lock.OwnerID != ownerID { | ||
| return false, nil |
There was a problem hiding this comment.
When the current primary stalls longer than PrimaryLockTTL, this renewal path still succeeds for the same owner because it checks only OwnerID and ignores whether ExpiresAt is already in the past. In a distributed pool, if the stale owner resumes just as another process is eligible to take over, it can extend the expired lease and publish warmups even though its lock window had already lapsed; reject renewals after expiry so leadership failover is actually time-bounded.
Useful? React with 👍 / 👎.
| // Check if in backoff | ||
| if p.recon.isBackoffActive() { | ||
| return | ||
| } |
There was a problem hiding this comment.
When warmup failures put this owner into backoff, this branch returns without renewing the primary lock. In a multi-process pool, the lock then expires during the backoff window and another owner with a fresh local reconcileState can acquire leadership and immediately resume create attempts, defeating the degraded backoff that is supposed to protect an unhealthy backend; renew the lock before returning here as the no-work branch does.
Useful? React with 👍 / 👎.
| // Stale sandbox; clean up | ||
| _ = p.config.StateStore.RemoveIdle(ctx, p.config.PoolName, sandboxID) | ||
| go p.killSandbox(sandboxID) |
There was a problem hiding this comment.
When connectAndValidate fails because the caller's acquire context is canceled or hits its deadline after TryTakeIdle has already removed the ID, this cleanup treats the candidate as stale and kills it. Under normal request timeouts, a healthy prewarmed sandbox can be destroyed and removed from the pool even though only the client-side acquire was canceled; distinguish ctx.Err()/deadline failures from stale sandbox failures before removing and killing the idle entry.
Useful? React with 👍 / 👎.
1 participant
Implement OSEP-0005 client-side pool for the Go SDK, enabling millisecond-level sandbox acquisition through pre-warmed idle instances.
Features:
Closes #1166
Summary
Features
SandboxPoolwith Start/Acquire/Resize/Shutdown/Snapshot/ReleaseAllIdleAcquirePolicy: DirectCreate (default) and FailFastTesting
Breaking Changes
Checklist
Closes #1166