Skip to content

Handle overflows in RespMemoryWriter - #1987

Merged
kevin-montrose merged 6 commits into
mainfrom
users/kmontrose/respMemoryWriterOverflow
Jul 24, 2026
Merged

kevin-montrose merged 6 commits into
mainfrom
users/kmontrose/respMemoryWriterOverflow

Conversation

@kevin-montrose

Copy link
Copy Markdown
Contributor

Fixes #1616
Supercedes #1945

Simplest possible fix (just changes the exception type and adds a message).
Includes tests for the RespMemoryWriter-using types (Hash, List, Set, and SortedSet) to prevent regressions.

Copilot AI review requested due to automatic review settings July 24, 2026 16:24

Copilot AI left a comment

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

test/standalone/Garnet.test/RespMemoryWriterOverflowTests.cs:115

  • In SetAsync/SortedSetAsync, building memberName via string interpolation copies the 40k-character longValue into a new string for every member. With 60,000 members this is several GB of managed allocations on the test client alone and is very likely to OOM the test runner before the server behavior is exercised.
            var longValue = new string('s', MemberLength);

            for (var i = 0; i < NumMembers; i += BatchSize)
            {
                var writeTasks = new Task<bool>[BatchSize];
                writeTasks.AsSpan().Fill(Task.FromResult(true));

                for (var j = 0; j < writeTasks.Length; j++)
                {
                    var memberName = $"{(i + j)}_{longValue}";
                    writeTasks[j] = db.SetAddAsync(Key, memberName);
                }

Comment thread test/standalone/Garnet.test/RespMemoryWriterOverflowTests.cs
@kevin-montrose
kevin-montrose merged commit ec98043 into main Jul 24, 2026
317 of 318 checks passed
@kevin-montrose
kevin-montrose deleted the users/kmontrose/respMemoryWriterOverflow branch July 24, 2026 18:05
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HGETALL Throw an exception when using on too many elements

3 participants