hash: Transfer waiting list ownership to the objcore #3992
Closed
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
This was referenced Sep 28, 2023
dridi
force-pushed
the
objcore_waitlist
branch
from
6dda666
to
235f17c
Compare
dridi
force-pushed
the
objcore_waitlist
branch
from
235f17c
to
87e1084
Compare
|
On Friday I took the original patch from this pull request and pushed it to the last mile. When I pushed the new patch series earlier I realized there was actually one more obvious thing to do (1f3b217). Since this was established as a priority last week during the VDD, and since this is not a trivial change, I hope it can be triaged during bugwash today so the review effort can start sooner than later. I will try to join the bugwash but connectivity is terrible in my train. |
|
Found more polish to apply after staring a little harder. |
It has nothing to do with the hash lookup implementation, this is the outcome of a cache lookup.
This is a prerequisite before moving waiting list ownership from objhead to objcore.
Adding expired variants of miss and hitmiss outcomes will enable better code organization, mainly the extraction of the objhead lookup step into its own function.
This creates a separate objhead lookup after the hash lookup, and a central location to process the objhead lookup result. This makes both the lookup code and the lookup result processing code more compact and easier to follow individually. In particular, the objhead lookup now operates entirely under the objhead lock and has a much better signal to noise ratio.
The advantages of the objhead owning the waiting list are that we need less waiting list heads as they are shared across multiple objcores, we get more frequent opportunities to rush waiting requests, and we avoid one expensive part of the lookup, which is to find an objhead from a request hash. The downsides are the requirement to always perform a new lookup, which increases contention on the objhead lock, and prevents expired but valid objects to be considered fresh. This is another way to describe the cache-control no-cache directive: a response considered fresh at fetch time, but immediately stale afterwards. Having the waiting list on the objcore means that we reenter the cache lookup with the object candidate from the previous lookup, so we may short-circuit the more expensive objhead lookup when the objcore is compatible with the request rushing off the waiting list. The expensive request hash lookup is still avoided, just like before, the objcore has a reference to the objhead. This creates a new first step in the lookup process: - rush match (when coming from the waiting list) - hash lookup (unless coming from the waiting list) - objhead lookup (unless there was a rush hit) This moves the infamous waiting list serialization phenomenon to the vary header match. Knowing that a rushed object is guaranteed to lead to a cache hit allows the rush policy to be applied wholesale, instead of exponentially. Only requests incompatible with the objcore vary header may reenter the waiting list, a scenario no different from the spurious rush wakeups when operating on the objhead. The waiting list operations are still guarded by the objhead lock. As it stands currently, the exponential rush is indirectly systematic, since the unusable objcore is rushed before the objhead lookup. This is the first step towards implementing no-cache and private support at the header field granularity.
Now that objcores own their waiting lists, they can no longer have anything waiting for them at creation time.
This has the same effect, with slightly less ceremony.
The objcore is officially referenced by a request when it leaves its waiting list during a rush. This allows migrating a waiting list from one objcore to the next without the need to touch reference counters. The hash_oc field will still have to be updated. This field must be be kept to navigate from a request to its waiting list to implement walking away from a waiting list. Refs varnishcache#3835
This is very useful to manually verify the rush_exponent behavior.
After the migration of the waiting list to the objcore, the result was that each request would rush more requests upon reembarking by dropping the waited objcore reference before the objhead lookup. The reference is now dropped after the objhead lookup, allowing for the lookup outcome to be checked, either triggerring a complete rush when the object is serviceable (modulus vary match) or moving the waiting list to the next busy objcore. This avoids the spurious wakeups caused by objhead rushes when waiting lists were owned by objheads. This is however a missed opportunity when there are multiple concurrent busy objcores. This could be alleviated by linking busy objcores together but at this point this is already a net improvement. The only way to get multiple concurrent busy objcores on the same objhead would be through the hash_ignore_busy flag anyway.
Now that waiting lists are owned by objcores, the rush policy no longer needs to leak everywhere. Either all the waiters are rushed at once for cacheable objects, or according to rush_exponent.
dridi
force-pushed
the
objcore_waitlist
branch
from
e19fca7
to
6fe0207
Compare
|
In the absence of a bugwash yesterday I took the liberty of rearranging the patch series one more time. |
|
Note from yesterday's bugwash: Dridi wants to take a slightly different approach and will come back. This is going to take some time. |
I started and completed the new iteration yesterday, but didn't take time to submit #4032. I believe that this time I cut all the waiting list loose ends and managed much better trade offs. |
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.
The advantages of the objhead owning the waiting list are that we need less waiting list heads as they are shared by across multiple objcores, we get more frequent opportunities to rush waiting requests, and we avoid one expensive part of the lookup, which is to find an objhead from a request hash.
The downsides are that the requirement to always perform a new lookup, which increases contention on the objhead lock, and prevents expired but valid objects to be considered fresh. This is another way to describe the cache-control no-cache directive: a response considered fresh at fetch time, but immediately stale afterwards.
Having the waiting list on the objcore means that we reenter the cache lookup with the object candidate from the previous lookup, so we may short-circuit the more expensive objhead lookup when the objcore is compatible with the request rushing off the waiting list. The expensive objhead lookup from the request hash is still avoided, the objcore has a reference to the objhead.
This moves the infamous waiting list serialization phenomenon to the vary header match. Knowing that a rushed object is guaranteed to lead to a cache hit allows the rush policy to be applied wholesale, instead of exponentially. Only requests incompatible with the objcore vary header may reenter the waiting list, a scenario no different from the spurious rush wakeups when operating on the objhead.
The waiting list operations are still guarded by the objhead lock.
This is the first step towards implementing no-cache and private support at the header field granularity.