Do not collapse ssl_crtd requests across reconfigurations

[eduard-bagdasaryan]

Squid collapses new ssl_crtd requests on a pending request with an identical helper query. This collapsing "works" across Squid reconfigurations even though the old helper responsible for the first request is replaced with a new one during reconfiguration; that old helper must complete servicing pending requests.

However, since the helper program itself could have been changed (just prior to reconfiguration), it is conceptually wrong for post-reconfiguration requests to reuse the old helper program response: Squid must conservatively assume that the response may have changed because the helper protocol does not allow Squid to validate the freshness of the helper response (to a collapsed request). Such blind reuse also creates runtime problems if a buggy helper never responds to a request X, stalling all the new requests getting collapsed on X (until a Squid restart).

The added squid.conf "tagging" or "versioning" scheme may be useful for Squid DNS and HTTP collapsing features that lack freshness checks and, hence, should not use potentially stale pending transactions as collapsing targets for new transaction after a reconfiguration.

Also pool GeneratorRequests map entries.

[rousskov]

This change essentially drops a major reason to have TheGeneratorRequests.

I disagree with that opinion. I see no connection between this change and the reason to have TheGeneratorRequests. The reason to have TheGeneratorRequests is to collapse security_file_certgen requests. Preventing collapsing in some special/rare cases (i.e. this change) does not drop the need for collapsing security_file_certgen requests (i.e. the reason to have TheGeneratorRequests).

Please instead merge this lists collapsing behaviour inside the class helper submit() method and queue member.

Adding generic support for collapsing helper requests is a major change that would affect functionality and performance of other existing helper types. Integration with existing helper-specific caching features increases that challenge. The terrible shape of the existing generic helper class hierarchy complicates the workspace further; we may need to fix that hierarchy (a large project on its own!) before adding such a heavy/complicated feature as request collapsing.

While generalizing helper caching/collapsing support may be a good idea, we should not block a bug-fixing PR for helper X just because it does not extend an existing helper X feature to other helpers, especially when extending that support is a very complex endeavor!

void
helper::submitRequest(Helper::Xaction *r)
{
   ...
    if ((srv = GetFirstAvailable(this)))
        helperDispatch(srv, r); // this request does not go through helper::queue
    else
        Enqueue(this, r);
    ...
}

On a technical side, the current helper::queue semantics cannot support collapsed requests because that queue does not see/keep all pending requests. In many use cases, that queue is always empty while the helper is very busy handling requests! Today, generic helper code keeps pending requests in 1+n queues: one helper::queue and n helper server queues (HelperServerBase::requests). A pending helper request belongs to one of those 1+n queues, not necessarily the helper::queue. That existing design itself creates certain problems, but adjusting it is difficult and way out of scope for this PR.

That will solve the problem targeted by this PR, the additional problem of how this list interacts with the helper queue, and add this collapsing optimization to all other helper types.

Moving the existing request collapsing feature from the certificate generation helper to generic helper code "as is" will not solve the problem targeted by this PR. The problem needs a dedicated fix. This PR provides such a fix. We may find an overall better fix, but the suggested move to "the class helper submit() method and queue member" is not it.

This PR does not even make extending/generalizing collapsing support in the future more difficult.

@yadij, please withdraw your blocking change request.