vcl_vrt: Skip VCL execution if the client is gone (6.0) #4006
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure why 6fbb937 does not originate from trunk.
It was particularly hard to follow once we reach client c3.
The goal is for top-level transports to report whether the client is still present or not.
Once a client is reportedly gone, processing its VCL task(s) is just a waste of resources. The execution of client-facing VCL is intercepted and an artificial return(fail) is returned in that scenario. Thanks to the introduction of the universal return(fail) proper error handling and resource tear down is already in place, which makes this change safe modulus unknown bugs. This adds a circuit breaker anywhere in the client state machine where there is VCL execution. A new Reset time stamp is logged to convey when a task does not complete because the client is gone. This is a good complement to the walk away feature and its original circuit breaker for the waiting list, but this has not been integrated yet. While the request is technically failed, it won't increase the vcl_fail counter, and a new req_reset counter is incremented. This new behavior is guarded by a new vcl_req_reset feature flag, enabled by default. Refs varnishcache#3835 Refs 61a15cb Refs e5efc2c Refs ba54dc9 Refs 6f50a00 Refs b881699 Conflicts: bin/varnishd/cache/cache_vrt_vcl.c bin/varnishd/cache/cache_vcl_vrt.c bin/varnishd/mgt/mgt_param_bits.c include/tbl/params.h
The error check is not performed in a critical section to avoid contention, at the risk of not seeing the error until the next transport poll. Conflicts: bin/varnishd/http2/cache_http2_session.c
|
Upon inspection, most of the failing tests were already registered in #4002 and a couple more very slow tests were added. One failing test is related to the change set and needs further inspection. |
|
The failing test that got me worried is the result of a missing back-port in #4004 of a patch that came after #3661. So I ended up troubleshooting the same bug twice with the same diagnostic, but I didn't even remember that I had already fixed it... I pushed the missing back-port here and this should be ready to merge once CI completes. |
|
This time, all of the failing tests were already registered in #4002 as slow, ready to merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a port of #3998 with b4c5030 folded in for the 6.0 branch.