Skip to content

Reintroduce progressive HTTP read timeout support - #3469

Open
zchuango wants to merge 1 commit into
apache:masterfrom
LinQuickDev:progressive-timeout-v2
Open

Reintroduce progressive HTTP read timeout support#3469
zchuango wants to merge 1 commit into
apache:masterfrom
LinQuickDev:progressive-timeout-v2

Conversation

@zchuango

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: Related to #3133, follow-up pr #3409 and #3453

Problem Summary:

Progressive HTTP response reads currently have no independent idle timeout after ReadProgressiveAttachmentBy() is called. A reader may therefore wait indefinitely when the peer stops sending body data while keeping the connection open.

PR #3409 introduced progressive-read timeout support, but post-merge CI exposed a lifecycle and synchronization problem around the timer callback and reader completion path. In particular, the timer callback could race with reader completion/error delivery, which made HttpTest.progressive_read_timeout_preserves_reader_error unstable and could replace the reader's original error with the timeout status.

This PR restores the feature with explicit synchronization and independent timer-state ownership.

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

Co-authored-by: zchuango <zchuang185@gmail.com>
@zchuango

Copy link
Copy Markdown
Contributor Author

@wwbmmm @chenBright Please carefully review this feature. We have conducted multiple rounds of tests both online and offline, and no abnormal issues have been found. For details, see the following test screenshots and evidence.

Additional validation evidence for commit fc865456fcc21f2e8325e3e9b0c27f7d6fce7b21.

Local stress testing

  • ASan: 200/200 rounds passed.
  • 6 focused progressive-read timeout tests per round.
  • 1,200 total test invocations.
  • 0 failures.
  • 0 sanitizer errors.
  • Bazel 8.3.1: 20/20 runs passed.
  • 120 total focused test invocations.

The attached archive contains:

  • sanitized ASan log;
  • sanitized Bazel log;
  • ASan reproduction script;
  • Bazel reproduction script;
  • SHA256 checksums for every included file.

Archive:

progressive-read-timeout-test-evidence-fc865456.zip

Archive SHA256:

3641592B51D1BCE19F5BA1796FB2F91C27E5980D94CC9E040847D16AF2CFBAA4

GitHub Actions reruns

Five of six complete Linux workflow attempts passed.

Attempt #5 failed only in the existing
clang-unittest-bazel-with-babylon-and-new-pb job, in
RdmaTest.v3_server_rejects_oversized_pb_size.

The other 10 Linux jobs in that attempt passed. This PR does not modify RDMA
code or that test. The subsequent complete attempt #6 passed all 11 Linux jobs,
including the previously failing job.

Validation Screenshots

ASan stress test: 200/200 rounds passed asan
Bazel 8.3.1 stress test: 20/20 runs passed bazel
GitHub Actions attempt #6: all Linux jobs passed CI

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.

2 participants