Skip to content

Fix an issue with updating data in redis #1708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jakubno wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix an issue with updating data in redis #1708

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/api/internal/sandbox/storage/redis/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (s *Storage) Update(ctx context.Context, sandboxID string, updateFunc func(
}

// Execute transaction
err = s.redisClient.Set(ctx, key, newData, redis.KeepTTL).Err()
err = s.redisClient.Set(ctx, key, newData, 0).Err()
Copy link

@claude claude bot Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting TTL to 0 means no expiration. Both Add() (line 29) and now Update() store keys without expiration, but the comment on line 28 says "with max expiration little bit longer than max instance length to prevent leaking". This means sandbox data will persist indefinitely in Redis unless explicitly removed via Remove(). If the cleanup logic fails, this could cause memory leaks. Consider either: (1) setting an actual TTL value (e.g., based on max instance lifetime), or (2) updating the comment to reflect that Redis is not being used as a TTL-based cache for sandboxes.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve TTL when updating sandbox entry

Using Set(..., 0) explicitly removes any existing TTL on the key. If sandbox entries ever have expirations set (e.g., via EXPIRE elsewhere or from prior versions), this update makes them persistent after any write, so they will never expire and can leak. The previous redis.KeepTTL preserved those expirations. Consider keeping TTLs (or reapplying the current TTL) when updating so updates don’t disable expiry for keys that have one.

Useful? React with 👍 / 👎.

if err != nil {
return sandbox.Sandbox{}, err
}
Expand Down
Loading