Re: [PATCH] Support client connection annotation by helpers via clt_conn_id=ID

Hi all,

The attached patch replaces existin annotation values with the new one
received from helpers.

Just one question. We are documenting key-value pairs in cf.data.pre
only for url-rewriter helpers, but annotations works for all squid helpers.
Should we move the related url-rewriter section to a global section? If
yes where?

For example something like the following in a global section should be
enough:

The interface for all helpers has been extended to support arbitrary
lists of key=value pairs, with the syntax key=value . Some keys have
special meaning to Squid, as documented here.

Currently Squid understands the following optional key=value pairs
received from URL rewriters:
  clt_conn_id=ID
     Associates the received ID with the client TCP connection.
     The clt_conn_id=ID pair is treated as a regular annotation but
     it persists across all transactions on the client connection
     rather than disappearing after the current request. A helper
     may update the client connection ID value during subsequent
     requests by returning a new ID value. To send the connection
     ID to the URL rewriter, use url_rewrite_extras:
     url_rewrite_extras clt_conn_id=%{clt_conn_id}note ...

On 06/19/2014 09:07 PM, Tsantilas Christos wrote:
> On 06/16/2014 06:36 PM, Alex Rousskov wrote:
>> On 06/15/2014 12:07 AM, Amos Jeffries wrote:
>>> On 15/06/2014 4:58 a.m., Alex Rousskov wrote:
>>>> On 06/11/2014 08:52 AM, Tsantilas Christos wrote:
>>>>
>>>>> I must also note that this patch adds an inconsistency. All annotation
>>>>> key=values pairs received from helpers, accumulated to the
>>>>> existing key
>>>>> notes values. The clt_conn_id=Id pair is always unique and replaces
>>>>> the
>>>>> existing clt_conn_id=Id annotation pair.
>>>>> We may want to make all annotations unique, or maybe implement a
>>>>> configuration mechanism to define which annotations are overwriting
>>>>> their previous values and which appending the new values.
>>>>
>>>> I suggest making all annotations unique (i.e., new values overwrite old
>>>> ones) because helpers that want to accumulate annotation values can do
>>>> that by returning old values along with new ones:
>>>>
>>>> received by helper: name=v1
>>>> returned by helper: name=v1,v2
>>>>
>>>> Please note that the opposite does not work: If annotation values are
>>>> always accumulated, a helper cannot overwrite/remove the old value.
>>
>>
>>> Doing that would mean passing all existing annotations to every helper
>>> lookup.
>>
>> Why would that mean the above?
>>
>> AFAICT, the helper should get only the annotations it needs. That need
>> is helper-specific and, hence, is configurable via the various _extras
>> and equivalent directives. That is already supported and does not need
>> to change.
>>
>> Here is the overall sketch for supporting "unique annotations":
>>
>> 1. Send the helper the annotations it is configured to get
>> (no changes here).
>>
>> 2. For each unique annotation key received from the helper,
>> remove any old annotation(s) with the same key.
>>
>> 3. Store annotations received from the helper
>> (no changes here).
>>
>> To support explicit annotation deletion, we can adjust #3 to skip
>> key-value pairs with the value equal to '-'.
>
> If there is not any objection I will implement this scenario.
>
> Looks that this approach is the best and cover more cases than the
> accumulated Notes values.
> If someones need to accumulate Note values he can configure squid to
> send old note value to helper and helper include it in its response.
> This is simple.
>
> If required in the future we can implement a configuration parameter
> which configures one or more notes as always accumulated. For example:
> AccumulateHelperNotes status message
>
>
>
>
>>
>>
>> HTH,
>>
>> Alex.
>>
>>
>
>