Skip to content

debug: test-only FullChunkCachedAfterPartialRequest#2125

Closed
levb wants to merge 12 commits intomainfrom
lev-debug-FullChunkCachedAfterPartialRequest-flapping
Closed

debug: test-only FullChunkCachedAfterPartialRequest#2125
levb wants to merge 12 commits intomainfrom
lev-debug-FullChunkCachedAfterPartialRequest-flapping

Conversation

@levb
Copy link
Copy Markdown
Contributor

@levb levb commented Mar 12, 2026

Failed to reproduce, removed the Eventually and added logging for slow results.

This is to debug the flaky behavior manifested here, https://github.com/e2b-dev/infra/actions/runs/23020326855/job/66854698657?pr=2120

levb and others added 2 commits March 12, 2026 15:33
Add error logging to the Eventually condition so CI failures show
whether Slice blocks (no logs) or returns persistent errors. Run both
the Eventually and direct variants 100 times each to increase the
chance of hitting the race in a single CI run.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd637c2b57

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ValentaTomas ValentaTomas removed their request for review March 12, 2026 23:44
@levb levb marked this pull request as draft March 15, 2026 11:37
levb and others added 8 commits March 15, 2026 04:38
…achedAfterPartialRequest

testify's Eventually runs conditions in goroutines, creating a race with
defer Close() when the timeout fires while the condition goroutine is
blocked in registerAndWait. Replace with a direct blocking Slice call
(which is what production uses) with an explicit 10s timeout context.

Also removes the eventually/direct dimension (200 → 100 subtests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…of github.com:e2b-dev/infra into lev-debug-FullChunkCachedAfterPartialRequest-flapping
…hunker

Same upstream, same flow (Slice first block → Slice last block), same
iteration count. If CI shows >1s warnings on streaming but not full-fetch,
the cause is background goroutine starvation, not general CI load.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Log first_slice and second_slice durations separately when >100ms so we
can identify exactly which phase is slow on CI. Lower WARNING threshold
from 1s to 100ms.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tests

Remove 100x loop, remove apples-to-apples timing test. Keep the fix:
replace flaky Eventually with direct blocking Slice, add 10ms slow
warning for visibility.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…erPartialRequest

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@levb levb marked this pull request as ready for review March 15, 2026 12:54
@levb
Copy link
Copy Markdown
Contributor Author

levb commented Mar 18, 2026

Looks like @dobrac merged it with one of his PRs already, no diff to main

@levb levb closed this Mar 18, 2026
@ValentaTomas ValentaTomas deleted the lev-debug-FullChunkCachedAfterPartialRequest-flapping branch March 20, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants