trafficserver-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bryan Call <bc...@yahoo-inc.com>
Subject Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig
Date Tue, 05 Nov 2013 00:46:29 GMT
+1

I think we should remove the code to be send these configuration options in a cluster request.
 Reason for this are:
1. These options are already propagated in cluster config
2. Cleaner code, able to remove code and simplify the cluster request
3. Seems odd to me to pass around a subset of the configuration to clusters per request and
isn't consistent with other configuration options

-Bryan

On Nov 3, 2013, at 12:27 PM, Leif Hedstrom <zwoop@apache.org> wrote:

> Hi all,
> 
> I’d like to get some closure on the TS-1919 Jira issue. There have been concerns raised
about this patch, and I will try to explain the pros and the cons of this patch. Note that
this is committed on the v5.0.x branch (it’s an incompatible change), and I really want
to decide the future of this commit such that we can either agree to keep it, or back it out
right now.
> 
> 
> Explanation of the patch.
> 
> TS-1919 eliminates a helper class named CacheLookupHttpConfig. This is used for the Cache
Clustering feature to transport 4 configurations between the cluster members, on every response.
This is to assure that both members of a negotiated transaction use the same value for those
4 configurations. The configurations passed along are:
> 
> 	proxy.config.http.cache.enable_default_vary_headers
> 	proxy.config.http.cache.vary_default_text
> 	proxy.config.http.cache.vary_default_images
> 	proxy.config.http.cache.vary_default_other
> 
> Instead of passing along the CacheLookupHttpConfig class while processing a request,
we now simply pass along the normal HTTP configuration (HttpConfigParams); this holds all
configurations, including the overridable configurations.
> 
> In addition to this, the helper class also encapsulated 5 additional configurations which
are necessary by the cache code, but the changes in TS-1919 doesn’t affect this; those configs
are still passed along, as will be explained further. However, *only* the four configs above
are transferred as part of the clustering messages. In fact, TS-1919 would make it possible
to make those 5 configurations overridable. In any case, these additional configs are
> 
> 	proxy.config.http.global_user_agent_header
> 	proxy.config.http.cache.ignore_accept_mismatch
> 	proxy.config.http.cache.ignore_accept_language_mismatch
> 	proxy.config.http.cache.ignore_accept_encoding_mismatch
> 	proxy.config.http.cache.ignore_accept_charset_mismatch
> 
> 
> Why I thought this was a useful patch?
> 
> There are a number of reasons why I thought this was necessary to change:
> 
> It much more cleanly separates the HTTP SM from the cache core. For example, this allows
us to eliminate the ifdef’s around HTTP_CACHE. The patch uses opaque void*’s when passing
module boundaries.
> Most importantly, it allows us to pass along the full, overridable Http txn config. This
means that some (but definitely not all) cache specific records.config parameters can now
be made overridable per remap rule or per transaction via APIs. This is a fairly frequently
requested RFE.
> It simplifies the cluster protocol, and reduces overhead around allocations, copying,
marshaling and unmarshalling the 4 configs above, as well as the other 5 configs that it has
to copy as part of the cache dependency.
> 
> 
> What are the downsides and concerns? Well, the one that was brought up was that being
able to pass along these 4 configurations across the clustering communications was mission
critical. I can’t speak for that view, since I don’t have that use case. The other major
downside is that this breaks compatibility, such that you can not mix a v4.x box with a 5.x
box in a cache cluster (they will split).
> 
> I feel that cluster configurations (broadcasted) solves most of the concerns in that
if you change e.g. Vary: default_text, it’ll get distributed within a few seconds. Also,
I feel that neither of the 4 settings are something you would change particularly frequently,
so in a very worst case scenario, you’d take the cluster out of rotation while pushing that
config change to all members.
> 
> Please discuss and voice your thoughts here. I’d really like to get a closure on this,
such that I can start implementing per transaction overridable cache configurations for v5.0.x
if we decide to keep TS-1919 as it is. In particular, if you have a strong “-1” opinion
on this commit as it is, please speak up now.
> 
> Cheers,
> 
> — Leif
> 


Mime
View raw message