Skip to content

Fix XPENDING empty-PEL reply schema and stop req-res-log-validator masking schema errors - #62

Open
madolson wants to merge 1 commit into
unstablefrom
ai/issue-49
Open

madolson wants to merge 1 commit into
unstablefrom
ai/issue-49

Conversation

@madolson

Copy link
Copy Markdown
Owner

XPENDING <key> <group> on a consumer group with an empty pending entries list replies [0, nil, nil, nil] (src/t_stream.c:3069-3072), but src/commands/xpending.json only described non-empty summaries, so reply-schemas-validator rejected a valid reply and failed the Daily run. The second, unrelated problem is that when any schema mismatch happens, utils/req-res-log-validator.py:239 re-raised the jsonschema.ValidationError from inside a multiprocessing.Pool worker; that exception holds a TypeChecker lambda in jsonschema._types and cannot be pickled, so the parent reported MaybeEncodingError: ... PicklingError and the real error was only visible 650 lines earlier in the log. This adds the empty-summary oneOf branch (backport of valkey-io/valkey#4653) plus a test that exercises it under --log-req-res, and raises a plain RuntimeError carrying the message so the validator's exit reports the actual mismatch.

Fixes #49. Schema half is a backport of valkey-io/valkey#4653; the validator masking is still present at upstream/unstable.

Details

Problem 1: schema does not describe the empty-PEL reply

src/t_stream.c:3064-3072:

    if (justinfo) {
        addReplyArrayLen(c, 4);
        /* Total number of messages in the PEL. */
        addReplyLongLong(c, raxSize(group->pel));
        /* First and last IDs. */
        if (raxSize(group->pel) == 0) {
            addReplyNull(c);      /* Start. */
            addReplyNull(c);      /* End. */
            addReplyNullArray(c); /* Clients. */

The summary branch of reply_schema declared items[1] and items[2] as type: string with pattern: [0-9]+-[0-9]+ and items[3] as type: array. Nothing accepted nulls, so [0, None, None, None] matched neither oneOf branch.

Before this change, no test on this branch called the two-argument form on an empty PEL. The only bare call is tests/unit/type/stream-cgroups.tcl:97, and its PEL has four entries. Upstream first hit this when valkey-io/valkey#4629 added such calls at tests/unit/type/stream.tcl:777 and :799, which is why exactly one Daily run failed.

Problem 2: the validator hides its own error

utils/req-res-log-validator.py:186 process_file runs in the pool created at :334. On mismatch, :230-239 printed diagnostics and then raised the jsonschema exception. Reproduced standalone with the pinned jsonschema==4.17.3:

PICKLE FAILS: PicklingError Can't pickle <function <lambda> at 0x7fa080229a70>: attribute lookup <lambda> on jsonschema._types failed
worker saw: None is not of type 'string'
PARENT SAW: MaybeEncodingError : Error sending result: '<multiprocessing.pool.ExceptionWithTraceback object at 0x7fa07f929ed0>'. Reason: 'PicklingError("Can't pickle <function <lambda> at 0x7fa080229a70>: attribute lookup <lambda> on jsonschema._types failed")'

This is not specific to XPENDING. Every reply-schema mismatch presents the same way, which is why automated fingerprinting of the failing run found no signature. Contrast valkey-io/valkey#2676, where the worker raised a ValueError and the parent reported it verbatim.

Alternative rejected

sys.exit(1) in the handler raises SystemExit, a BaseException. multiprocessing.pool.worker only catches Exception, so the worker would die without sending a result and pool.map can block. RuntimeError(str(err)) propagates through the pool normally and preserves the message. The full jsonschema diagnostics were already printed by :231-238 and are unaffected.

Testing

Built with make SERVER_CFLAGS='-Werror -DLOG_REQ_RES', ran ./runtest --single unit/type/stream-cgroups --log-req-res --force-resp3 --dont-clean, then ./utils/req-res-log-validator.py.

Without either fix, with the new test present, both defects show at once. The validator exits 1, and the tail is the misleading error:

On instance:
    [0, None, None, None]

Traceback (most recent call last):
  File "./utils/req-res-log-validator.py", line 337, in <module>
    for result in pool.map(func, paths):
  File "/usr/lib64/python3.7/multiprocessing/pool.py", line 268, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
  File "/usr/lib64/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
multiprocessing.pool.MaybeEncodingError: Error sending result: '<multiprocessing.pool.ExceptionWithTraceback object at 0x7f4d9e7052d0>'. Reason: 'PicklingError("Can't pickle <function <lambda> at 0x7f4da0610170>: attribute lookup <lambda> on jsonschema._types failed")'

The real cause was 46 lines earlier:

JSON schema validation error on /tmp/wt-49/tests/tmp/server.4034.1/stdout.reqres: [0, None, None, None] is not valid under any of the given schemas
argv: ['XPENDING', 'mystream', 'emptygroup']

With only the validator fix applied and the old schema still in the binary, the same logs produce the real error at the tail:

Traceback (most recent call last):
  File "./utils/req-res-log-validator.py", line 341, in <module>
    for result in pool.map(func, paths):
  File "/usr/lib64/python3.7/multiprocessing/pool.py", line 268, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
  File "/usr/lib64/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
RuntimeError: JSON schema validation error on /tmp/wt-49/tests/tmp/server.4034.1/stdout.reqres: [0, None, None, None] is not valid under any of the given schemas

With both fixes, the validator exits 0 and reports xpending: 33 hits.

This was generated by AI but verified, with love, by a human.

…rors

XPENDING <key> <group> on an empty PEL replies [0, nil, nil, nil]
(src/t_stream.c:3069-3072), but src/commands/xpending.json only described
non-empty summaries, so reply-schemas-validator rejected a valid reply. This
adds the empty-summary oneOf branch (backport of #4653) and a test that
exercises it under --log-req-res.

Separately, utils/req-res-log-validator.py re-raised the jsonschema
ValidationError from inside a multiprocessing pool worker. That exception
holds a TypeChecker lambda in jsonschema._types, which cannot be pickled, so
the parent reported MaybeEncodingError/PicklingError instead of the schema
mismatch. Raise a plain RuntimeError carrying the message instead.

Signed-off-by: Madelyn Olson <matolson@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[daily-ci] REAL-DEFECT: XPENDING empty-PEL reply schema failed reply-schemas-validator; validator masks the error as a PicklingError

1 participant