Skip to content

fix(spans): Avoid unpack() crashes when merging sets #113442

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

Merged
untitaker merged 1 commit into from
Apr 20, 2026
Merged

fix(spans): Avoid unpack() crashes when merging sets #113442

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
37 changes: 28 additions & 9 deletions src/sentry/scripts/spans/add-buffer.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,28 @@ RETURNS:

local project_and_trace = KEYS[1]

-- Lua's unpack() has a stack limit (typically ~7998 elements in Lua 5.1).
-- When merging member-keys sets that have accumulated across many EVALSHA calls,
-- we use SSCAN to stream members in batches to avoid both memory issues from
-- large smembers results and "too many results to unpack" errors.
local SCAN_BATCH_SIZE = 1000

local function merge_set(source_key, dest_key)
local cursor = "0"
repeat
local result = redis.call("sscan", source_key, cursor, "COUNT", SCAN_BATCH_SIZE)
cursor = result[1]
local members = result[2]
if #members > 0 then
-- we do not use SUNIONSTORE here because we assume the target set
-- at dest_key can be massive. if it is, SUNIONSTORE will copy the
-- entire target set again, which appears to be _worse_ than
-- copying the source set into lua memory and out again.
redis.call("sadd", dest_key, unpack(members))
end
until cursor == "0"
end

local num_spans = ARGV[1]
local parent_span_id = ARGV[2]
local has_root_span = ARGV[3] == "true"
Comment on lines +60 to 70

@sentry sentry Bot Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The unpack(hset_args) call in the Lua script may exceed the argument limit if num_spans is large, as there is no explicit chunking for the hset operation.
Severity: HIGH

Suggested Fix

Apply a similar batching mechanism to the hset call as was done for the sadd calls in the same script. This involves iterating over the arguments in chunks to avoid exceeding the Lua stack limit when calling unpack.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/scripts/spans/add-buffer.lua#L43-L70

Potential issue: The Lua script at `src/sentry/scripts/spans/add-buffer.lua` uses
`redis.call("hset", ..., unpack(hset_args))`. The `unpack` function has a stack limit of
around 8000 arguments. Since `hset_args` contains two entries per span, if the number of
spans exceeds approximately 3999, the call will fail with a stack overflow, causing a
runtime crash. While other calls like `sadd` were updated to handle large numbers of
arguments by batching, this `hset` call was not, despite the `max_spans_per_evalsha`
setting defaulting to an unlimited value (0). This appears to be an oversight.

Also affects:

  • src/sentry/scripts/spans/add-buffer.lua:124

Did we get this right? 👍 / 👎 to inform future reviews.

@untitaker untitaker Apr 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we limit EVALSHA on the caller side

Expand Down Expand Up @@ -150,9 +172,8 @@ if salt ~= "" then
end

local child_members_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, span_id)
local child_members = redis.call("smembers", child_members_key)
if #child_members > 0 then
redis.call("sadd", members_key, unpack(child_members))
if redis.call("exists", child_members_key) == 1 then
merge_set(child_members_key, members_key)
redis.call("del", child_members_key)
end
end
Expand Down Expand Up @@ -191,9 +212,8 @@ for i = NUM_ARGS + 1, NUM_ARGS + num_spans do
local span_id = ARGV[i]
if span_id ~= parent_span_id then
local child_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, span_id)
local child_members = redis.call("smembers", child_mk_key)
if #child_members > 0 then
redis.call("sadd", member_keys_key, unpack(child_members))
if redis.call("exists", child_mk_key) == 1 then
merge_set(child_mk_key, member_keys_key)
redis.call("del", child_mk_key)
end
end
Expand All @@ -202,9 +222,8 @@ end
-- Merge parent's tracking set if parent_span_id redirected to a different root.
if set_span_id ~= parent_span_id then
local parent_mk_key = string.format("span-buf:mk:{%s}:%s", project_and_trace, parent_span_id)
local parent_members = redis.call("smembers", parent_mk_key)
if #parent_members > 0 then
redis.call("sadd", member_keys_key, unpack(parent_members))
if redis.call("exists", parent_mk_key) == 1 then
merge_set(parent_mk_key, member_keys_key)
redis.call("del", parent_mk_key)
end
end
Expand Down
Loading