adapter: fix the XREAD wrong-type test's shared timing budget by bootjp · Pull Request #1249 · bootjp/elastickv

bootjp · 2026-09-14T08:41:48Z

TestRedis_StreamXReadBlockChecksWrongTypeAtDeadline is timing-dependent and took down test (ubuntu-latest) on #1222, which touches only internal/snapshotoffload.

Mechanism

The test blocked for 2 s and waited up to 2 s for the stream waiter to register before overwriting the key:

Block: 2 * time.Second,
...
requireStreamWaiterRegistered(...)   // require.Eventually, 2s timeout
require.NoError(t, rdbWriter.Set(ctx, key, "now-a-string", 0).Err())

The wrong-type check runs when the block deadline fires, so a SET that lands after it gets the ordinary block-timeout nil instead:

Error: "redis: nil" does not contain "WRONGTYPE"

Registration and the remaining block window drew on the same 2 s, so a loaded machine could consume the whole window before the SET was sent.

Reproduced, not guessed

Delaying the SET past the deadline produces the identical failure:

--- FAIL: TestRedis_StreamXReadBlockChecksWrongTypeAtDeadline (3.89s)
    Error: "redis: nil" does not contain "WRONGTYPE"

Fix

Block window 6 s, registration wait 1 s — the two no longer share a budget. The registration timeout becomes a parameter, because any caller whose assertion depends on acting before the reader's deadline has to bound it well under that deadline, and it now carries its own failure message, so a slow registration fails as "the waiter never appeared" rather than surfacing later as a confusing nil read.

Risk

None to production: test-only, no production file touched. The test's subject — that the wrong-type check happens at the deadline — is unchanged; only the margin around it is.

Test evidence

go test ./adapter/ -run TestRedis_StreamXReadBlockChecksWrongTypeAtDeadline -count=5 — ok
With a 2.5 s delay injected before the SET (the condition that broke it): passes at the new 6 s window, and fails with the original 2 s window — so the widened window is what does the work, not the injected delay.
golangci-lint --config=.golangci.yaml run ./adapter/... — 0 issues

Self-review

Data loss — none; test-only.
Concurrency / distributed failures — removes a timing dependency. The registration wait is still bounded, so a genuinely broken waiter registration still fails, now with a clearer message.
Performance — the test takes ~6 s instead of ~2 s in the deadline path. It is t.Parallel(), so the cost to the suite is minimal.
Data consistency — untouched; the wrong-type detection path is unchanged.
Test coverage — same assertions, now deterministic under load.

Fourth timing-dependent test flake in this batch (#1236 carries three others). All four share a shape: an assertion pinning one of several equally-correct outcomes, or a budget shared between setup and the thing being measured.

@codex review
@claude review

https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE

…tration TestRedis_StreamXReadBlockChecksWrongTypeAtDeadline blocked for 2s and waited up to 2s for the stream waiter to register before overwriting the key. The wrong-type check runs when the block deadline fires, so a SET that lands after it gets the ordinary block-timeout nil instead -- which the test reports as "redis: nil" does not contain "WRONGTYPE" On a loaded machine the registration wait can consume the whole window before the SET is sent. Reproduced exactly by delaying the SET past the deadline. The block window is now 6s and the registration wait 1s, so the two no longer share a budget: a slow registration fails as "the waiter never appeared", with its own message, rather than surfacing later as a confusing nil read. The registration timeout is a parameter now, since any caller whose assertion depends on acting before the reader's deadline has to bound it well under that deadline. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE

coderabbitai · 2026-09-14T08:41:56Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 28583a89-36d6-4461-a6f0-6f32fa572ec9

📥 Commits

Reviewing files that changed from the base of the PR and between 16b56f2 and 0625c57.

📒 Files selected for processing (1)

adapter/redis_compat_commands_stream_test.go

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector · 2026-09-14T08:42:04Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-22T17:25:31.917159Z	`0625c57`	Manual request
🔒 Security Review	✅ Completed 2026-10-22T17:26:27.327204Z	`0625c57`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1e10274af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

bootjp · 2026-09-15T14:29:34Z

CI の test (ubuntu-latest) 失敗はこの PR の変更とは無関係でした。TestGRPCS3BlobClusterAuthenticatedCapabilityPushAndFetch が codes.Unauthenticated を期待して codes.Unknown を受け取っていたもので、原因は main に元からあるクライアントストリームのバグです。

gRPC はサーバ側が既に終了させたストリームへの Send を素の io.EOF として返し、実際の status は受信側 (CloseAndRecv) からしか読めません。PushChunkBlob を含む 4 箇所がその io.EOF をそのまま呼び出し元へ返していたため、reset が Send に競り勝った実行でだけ status が失われます。テストの flake ではなく本番のバグで、s3_blob_replicator.go:136 のリトライ判断がこのエラーを見ています。

#1252 で 4 箇所すべてを修正しました。マージ後に main を取り込めばこの CI は緑になるはずです。

Copilot

🟢 Approval recommended

The test-only timing adjustment is consistent, bounded, and updates all helper call sites.

0 open findings

What changed in this PR

Stabilizes the Redis XREAD wrong-type deadline test by separating registration and block timing budgets.

Changes:

Extends the block window and adds a bounded waiter-registration timeout.
Improves timeout diagnostics and helper configurability.

File	Description
`adapter/redis_compat_commands_stream_test.go`	Makes the deadline-sensitive test more reliable under load.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

bootjp · 2026-10-22T17:23:07Z

Please review current HEAD 0625c57 after its latest push. Merge will wait for a passing review of this exact HEAD and successful CI. @codex review

chatgpt-codex-connector · 2026-10-22T17:25:30Z

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 0625c57891

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-22T17:26:25Z

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 0625c57891

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector Bot reviewed Sep 14, 2026

View reviewed changes

Comment thread adapter/redis_compat_commands_stream_test.go Outdated

bootjp mentioned this pull request Sep 15, 2026

adapter,raftengine: read the real status off a reset client stream #1252

Merged

Merge main into fix/redis-xread-block-deadline

a23db87

Copilot AI balanced review requested due to automatic review settings October 11, 2026 10:44

adapter: Allow time for XREAD waiter registration

44af495

Copilot started reviewing on behalf of bootjp October 11, 2026 10:56

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

Merge commit '16b56f2d7a8c184e5ffba8faeb9a7df477707ff9' into HEAD

0625c57

bootjp merged commit 9de16af into main Oct 22, 2026
9 checks passed

bootjp deleted the fix/redis-xread-block-deadline branch October 22, 2026 17:28

Navigation Menu

Repository navigation

adapter: fix the XREAD wrong-type test's shared timing budget - #1249

adapter: fix the XREAD wrong-type test's shared timing budget#1249
bootjp merged 4 commits into
mainfrom
fix/redis-xread-block-deadline

bootjp commented Sep 14, 2026

Uh oh!

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

bootjp commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bootjp commented Oct 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bootjp commented Sep 14, 2026

Mechanism

Reproduced, not guessed

Fix

Risk

Test evidence

Self-review

Uh oh!

coderabbitai Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

bootjp commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

bootjp commented Oct 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026

🛡️ Codex Security Review · Automatically triggered

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 14, 2026 •

edited

Loading