On 7/11/23 03:35, Francesco Chemolli wrote:
I'd like to start working on transitioning ipcache and fqdncache to
ClpMap.
Thank you. Please _plan_ to convert them both (as you are already doing
here!), but then convert them one at a time (to avoid duplicating
review/modification efforts). I bet we will need at least 6 "minimal"
PRs here.
There is only one snag,
FWIW, I did not check all the details, but I see two more snags,
including a very important/difficult one:
2. ipcache.cc code "locks" entries to prevent their removal from the
cache in certain cases. See ipcache_entry::locks. The corresponding code
already has serious problems, but the approach itself needs to be
refactored using RefCount, so that a cache entry can be removed from the
cache at any time. This is not going to be easy and will require at
least one dedicated PR, but I believe it is an essential preliminary step.
3. We should also remove ipcache_low and ipcache_high directives before
the conversion. Those two parameters do not make sense for an
instant-removal cache: They are not needed for anything useful, create
performance anomalies, and increase code/configuration complexity. The
corresponding logic is not supported by ClpMap.
Now back to your snag #1:
the current configuration directives specify a cache
size in number of entries, where ClpMap specifies a max size.
I can see two ways forward:
1. Add a second max-size parameter to ClpMap, ensuring that it starts
expunging entries when the maximum capacity in either memory use or
number of entries is reached
I am not against this option, especially if we deprecate/remove existing
count-based configuration (ipcache_size and friends) and add byte-based
configuration to replace it (the directive name is to be determined).
2. guesstimate how many bytes of memory an ipcache/fqdncache uses, and
convert
I support this option if we deprecate/remove existing counter-based
configuration (ipcache_size and friends) and add bytes-based
configuration (the directive name is to be determined). I prefer this
option in this case because it avoids making ClpMap code more complex
long-term just to accommodate a short-term deprecation need. The extra
code complexity is not huge, but cache limit enforcement is tricky, and
the history of the related ClpMap code suggests that adding a new limit
vector will take several (likely painful) iterations.
Regardless, I believe that from the user-facing perspective, a solid way
forward is to give to the ipcache_size and fqdncache_size directives an
option to specify a max size in entries or, by adding a memory-size
suffix, in used memory, and possibly deprecate the max-size-in-entries
option in Squid 7 and retire it in squid 8.
In the vast majority of deployments, the existing count-based limit is
inferior to the byte-based limit because memory bytes is a real resource
that admins understand and have to optimize/ration/limit (while the
number of entries has no direct relationship to anything most admins
should care about). Thus, long-term, we should provide byte-based
configuration options.
To keep things simple and to facilitate deprecation, I recommend adding
two new directives instead of making the existing directives more
complex. Naming is a separate/secondary issue, but we can model the new
names based on the two existing primary caches (cache_dir and
cache_mem). Alternatively, we can use a more natural(?) word order.
Here are a few variants (from most to least preferred by me):
* dns_cache and reverse_dns_cache
* cache_dns and cache_dns_reverse
* cache_ip and cache_domain
HTH,
Alex.
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
