[DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Leif Hedstrom
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:

  1. 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.
  2. 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.
  3. 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

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Reindl Harald


Am 03.11.2013 21:27, schrieb Leif Hedstrom:
> if you have a strong “-1” opinion on this commit as it is, please speak up now

i do not have a strong opinion because no clustering here this time

but i would like to say thank you for this attitude introducing changes
if any developer out there would act this way the world would be a better
for sysadmins and more time for a relaxed beer or two!


signature.asc (271 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Bryan Call
In reply to this post by Leif Hedstrom
+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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Leif Hedstrom
In reply to this post by Leif Hedstrom

On Nov 3, 2013, at 1:27 PM, Leif Hedstrom <[hidden email]> 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.

Haven’t heard any -1’s on this topic so far, so last call. Any concerns with keeping this change as it currently applies to in the v5.0.x ?

Thanks,

— leif

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Yongming Zhao
Leif: as we have talked, we are using that feature, and the late read-while-writer enhancement(not yet submit due to it is bound to our cluster refine) is also built onto this structure. can we keep it?

afaik, the CacheLookupHttpConfig is a control from http cache layer that will permit flex control before the cache is really done, as the http & cache is designed to be two separated  layer, that sounds not a too bad idea for me.

-1, that is my concern, cluster is the feature I can not compromise.

thanks

在 2013年11月12日,下午11:53,Leif Hedstrom <[hidden email]> 写道:

>
> On Nov 3, 2013, at 1:27 PM, Leif Hedstrom <[hidden email]> 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.
>
> Haven’t heard any -1’s on this topic so far, so last call. Any concerns with keeping this change as it currently applies to in the v5.0.x ?
>
> Thanks,
>
> — leif
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] TS-1919: Eliminate CacheLookupHttpConfig

Leif Hedstrom
On Nov 12, 2013, at 10:41 PM, Yongming Zhao <[hidden email]> wrote:
>
> Leif: as we have talked, we are using that feature, and the late read-while-writer enhancement(not yet submit due to it is bound to our cluster refine) is also built onto this structure. can we keep it?
>
> afaik, the CacheLookupHttpConfig is a control from http cache layer that will permit flex control before the cache is really done, as the http & cache is designed to be two separated  layer, that sounds not a too bad idea for me.

I think you need to reread the mails and the patch. This behavior is not changing, in fact it's getting much stronger. Instead of just 4 configs being transferred over cluster communications, all of them are, including the overridable configs. And this is communicated between HTTP and layers cache on the node.

I think from what you describe you need, TS-1919 is a major improvement, so there must be some major communications problem here.

>
> -1, that is my concern, cluster is the feature I can not compromise.

Sigh, ok. Can you please read the emails and patch once more, and if you still -1 in, I will remove it. The only thing you are missing in the patch is the transport of those four configs over cluster communications, and you probably never change those configs (they are not overrideable before this patch, but after the patch they could be).

Leif




>
>> 在 2013年11月12日,下午11:53,Leif Hedstrom <[hidden email]> 写道:
>>
>>
>>> On Nov 3, 2013, at 1:27 PM, Leif Hedstrom <[hidden email]> 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.
>>
>> Haven’t heard any -1’s on this topic so far, so last call. Any concerns with keeping this change as it currently applies to in the v5.0.x ?
>>
>> Thanks,
>>
>> — leif
>>