Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

ws: Prep work for trailers support - #4130

Merged
dridi merged 2 commits into
varnishcache:masterfrom
dridi:ws_trailers_prep
Jul 8, 2024
Merged

dridi merged 2 commits into
varnishcache:masterfrom
dridi:ws_trailers_prep

Conversation

@dridi

@dridi dridi commented Jul 3, 2024

Copy link
Copy Markdown
Member

Originally from #4125, submitted independently as per bugwash to reduce the size of #4125.

dridi and others added 2 commits July 3, 2024 14:38
This puts the caller in charge of triggering the workspace rollback
before moving pipelined bytes around. This will open the door to
pipelining for req and beresp trailers.

The workspace emulator version of WS_ReqPipeline() had a systematic
rollback instead of a conditional one. The reason why this didn't pose
a problem was that fetch tasks never need pipelining and always take
the shortcut where the rollback was properly optional.

This is a good occasion to simplify the emulated variant and document
why it is so different from the original.

Co-authored-by: Walid Boudebouda <walid.boudebouda@gmail.com>
The reset pointer is effectively the beginning of the reservation, so in
order to better generalize HEADERS frames processing between headers and
trailers, we reference the workspace directly.
@nigoroll

nigoroll commented Jul 8, 2024

Copy link
Copy Markdown
Member

The second (hpack) commit seems to make sense to me, but I do not understand the first: What are your plans with the pipeline logic? #4125 does not seem to make use of the conditional rollback and I do not understand what the meaning of a "pipeline at the end of the workspace" would be?

@dridi

dridi commented Jul 8, 2024

Copy link
Copy Markdown
Member Author

What are your plans with the pipeline logic? #4125 does not seem to make use of the conditional rollback

The new condition for the rollback ended up in this pull request, in HTC_RxInit().

I do not understand what the meaning of a "pipeline at the end of the workspace" would be?

I can't find a reference to the text you quoted.

@dridi

dridi commented Jul 8, 2024

Copy link
Copy Markdown
Member Author

What are your plans with the pipeline logic?

I forgot to answer this specific question.

Fetching trailers is done with the same building blocks that fetch headers. And they feed from pipelined bytes before fetching more. Your observation is on point, the pull request currently doesn't leave any room for pipelining, because chunked framing is still fetched one byte at a time.

Moving away from this suboptimal (euphemism) mode of operations is definitely a goal. I opted for status quo, because that reduced the amount of changes for the first step.

@nigoroll

nigoroll commented Jul 8, 2024

Copy link
Copy Markdown
Member

bugwash: I still do not understand how the first patch is going to be used later, because #4125 does not currently use it, IIUC. But I do not want to hold things up.

@dridi

dridi commented Jul 8, 2024

Copy link
Copy Markdown
Member Author

It prevents HTC_RxInit() from rolling back a req workspace for trailers.

bugwash: merge and rebase #4125 onto trunk.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants