2042357 - ZeroRttHandle should not seek on success

Status: NEW

(Core :: Networking: HTTP, enhancement, P2)

Description

[Valentin Gosu [:valentin]]

Currently ZeroRttHandle seeks the stream to the last read byte.
I think this is not really necessary, as we could have the ZeroRttHandle hold a buffer of the data it's already read from the transaction. If accepted, it just continues reading. If rejected, it seeks to the begining.

The current pattern of seeking to different places in the transaction seems quite error prone, and definitely doesn't work with fetch streaming upload or my work in bug 1971997.

Comment 1

[Valentin Gosu [:valentin]]

Attachment: WIP: Bug 2042357 - Replace per-racer stream seeking in 0-RTT with a shared replay buffer r=#necko

Introduce mEarlyDataBuffer on ZeroRttHandle and rewrite
ZeroRttHandle::ReadSegments around it: racers replay from the buffer
first, and only when caught up to its high-water mark pull more bytes
from the real txn's request stream (which are appended back to the
buffer for later racers). This removes the per-read Seek that
previously juggled the shared stream cursor between racers, and lets
us drop the "stream not seekable -> bail" branch entirely.

Finish0RTT's accept-path seek becomes conditional: the stream cursor
naturally sits at mEarlyDataBuffer.Length(), so a seek-back is only
needed in the edge case where a non-winning racer pulled past the
winner's offset. The reject path is unchanged
(FinishAdopted0RTT(restart=true) rewinds to 0).

Comment 2

[Valentin Gosu [:valentin]]

Attachment: WIP: Bug 2042357 - Designate a single puller for the 0-RTT replay buffer to drop the accept-path seek r=#necko

The first racer to reach the buffer's high-water mark is recorded as
mPuller; only that racer is allowed to extend mEarlyDataBuffer.
ReadSegments takes the caller HET so it can enforce this. With every
racer's offset bounded above by what the puller has pulled, the
accept-path Seek in Finish0RTT can be dropped -- the real txn just
continues reading from the stream cursor (= mEarlyDataBuffer.Length())
post-adopt. The reject path still rewinds via FinishAdopted0RTT.

The nsISeekableStream include is no longer needed.

Comment 3

[Valentin Gosu [:valentin]]

Attachment: WIP: Bug 2042357 - Move the 0-RTT replay loop into HappyEyeballsTransaction r=#necko

The per-racer offset (m0RttRequestStreamOffset) lives on the HET, so
the replay loop belongs there too. ZeroRttHandle now exposes two
narrow primitives:

• EarlyDataBuffer() -- Span<const uint8_t> of the shared prefix
• PullEarlyData(aCaller, reader, ...) -- puller-gated stream pull

HappyEyeballsTransaction::Read0RTTSegments owns the offset, drives the
replay against EarlyDataBuffer(), calls PullEarlyData when it catches
up, and updates the real txn's EARLY_SENT disposition. The
Maybe<uint64_t>& out-param on the old ReadSegments entry point is
gone.

Comment 4

[Valentin Gosu [:valentin]]

Attachment: WIP: Bug 2042357 - Drop the puller/non-puller distinction in ZeroRttHandle r=#necko

The puller restriction capped how fast mEarlyDataBuffer grew but
didn't actually prevent the seek-back case (a winner whose offset
trails the buffer high-water still produced the same situation), so
it was paying complexity for no behavioural benefit. Any racer caught
up to the high-water mark can now extend the shared buffer; pulled
bytes are still appended in order and replayed by all racers.

PullEarlyData no longer needs the aCaller argument, and mPuller is
removed.

Comment 5

[Valentin Gosu [:valentin]]

Kershaw, let me know what you think of the general approach here.
I really think we ought to simplfy the ZeroRttHandle a bit.

Comment 6

[Kershaw Chang [:kershaw]]

(In reply to Valentin Gosu [:valentin] from comment #5)

Kershaw, let me know what you think of the general approach here.
I really think we ought to simplfy the ZeroRttHandle a bit.

I totally agree with you. We should make ZeroRttHandle simpler.
I only looked at the first patch and I think it's a better approach. Thanks!