Add json error#25
Open
elad-ash wants to merge 2 commits into
Open
Conversation
4 tasks
fcostaoliveira
added a commit
that referenced
this pull request
Jun 2, 2026
…and tracking (#435) * fix(resp3): accept push frames; count nil _ as miss in arbitrary command tracking Two RESP3 correctness bugs surfaced by the 2.4 readiness review. #27: aggregate_type() lacked '>'. A server-pushed message (pubsub, keyspace notifications, client tracking invalidation) on a RESP3 connection hit the "unsupported response" path and dropped the connection. Now accepts and drains push frames inline. #25: SingleNullBulk and ArrayPerElementNulls miss sentinels only recognised RESP2 nil shapes ($-1/*-1/*0). RESP3 nil (_\r\n) was miscounted as a hit, silently understating miss rate and suppressing --miss-rate-threshold warnings. Refs: 2.4 review findings #25 + #27 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(resp3): flag RESP3 null at construction; distinguish from literal "_" Bugbot flagged that the ArrayPerElementNulls walker used a content heuristic (value_len==1 && value[0]=='_') that couldn't tell a RESP3 null parsed via single_type from a real bulk value of literal "_" parsed via blob_type. Both produce identical bulk_el. Added bulk_el::is_resp3_null flag, set only when single_type constructs the element for type byte '_'. Walker now reads the flag directly instead of inferring from content. Test added that stores "_" values and asserts they are NOT counted as misses. Refs: bugbot LOW on PR #435 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(resp3): drain server-pushed frames without delivering as reply Reviewer caught that the prior commit accepted RESP3 push (>) into aggregate_type() but didn't suppress its effect on m_last_response. The push frame fully parsed AS a reply, response_ended() returned 1, and memtier dequeued the push as the reply to the next in-flight command -- corrupting latency attribution + hit/miss accounting. Adds an m_push drain-mode flag: - Set when a push aggregate is encountered at top level - Suppresses every mutation to m_last_response while true - Cleared in response_ended(), which returns false so the parser continues to read the actual reply - Reset on rs_initial transition Refs: 2.4 review of PR #435 (R1 CRITICAL). * fix(resp3): reset m_response_len when draining push frame Bugbot caught that response_ended()'s m_push drain returned false without resetting m_response_len. The accumulated push frame bytes stayed in the counter and contaminated set_total_len() for the actual reply, skewing bandwidth and per-op size statistics. Resetting m_response_len to 0 at the push-drain-complete site matches the behaviour at rs_initial (the only other site where m_response_len is reset). Refs: bugbot LOW on PR #435 (follow-up). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(resp3): clear both m_push and m_attribute together in response_ended() Bugbot caught that if an attribute (|) appears nested inside a push frame (>), and both complete at the same response_ended() call, the m_attribute branch fired first and returned false, leaving m_push set. The parser then consumed the actual command reply as push-drain data, desynchronizing the connection. response_ended() now clears whichever flags are set in a single call and returns false if either was set. Refs: bugbot LOW on PR #435 (follow-up). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding errors prints to JSON in case of exit(1) and cleaning debug prints from JSON file writing