On 2014-03-12 12:12, Alex Rousskov wrote:
The patch pasted below is related to a long discussion on squid-users.
See the "ICP and HTCP and StoreID" thread there.
On 02/14/2014 04:38 AM, Nikolai Gorchilov wrote:
On Fri, Feb 14, 2014 at 7:22 AM, Alex Rousskov wrote:
On 02/13/2014 06:05 PM, Nikolai Gorchilov wrote:
Current implementation prevents the refresh.
We know that the current implementation is broken, no questions about
it! IIRC, the developer responsible for that breakage promised to fix
it
when the code with a known breakage was committed, but even if he
does
not, Amos, I, or others will [eventually].
I got the attention of one of my team mates regarding this.
Unfortunately he didn't have enough time to comprehend the complete
Squid logic, thus the ugliness of his patch. If you provide me with
high-level design advise for a proper fix, we may try to produce one.
Some time passed since the above exchange, and Niki has tested the
following patch. It helped when Squid was forwarding a request to a
peer
after a UDP_HIT for a StoreID-mapped object.
--- src/http.cc 2014-03-07 15:40:37.945321671 +0200
+++ src/http.cc 2014-03-07 15:41:36.853321533 +0200
@@ -2106,7 +2106,7 @@
Http::ProtocolVersion httpver(1,1);
const char * url;
if (_peer && !_peer->options.originserver)
- url = entry->url();
+ url = urlCanonical(request);
else
url = request->urlpath.termedBuf();
mb->Printf("%s %s %s/%d.%d\r\n",
The patch makes sense to me: StoreEntry::url() may have nothing to do
with the URL, just like the old MemObject::url() which is already
renamed to MemObject::storeId(). The patch does not fix all the
problems
discussed on squid-users, but it also does not make any of them worse
AFAICT.
I have not tested the above patch, have not considered alternatives,
and
do not want to become responsible for this Store ID bug or its fix at
this time.
If there are no objections or volunteers to properly introduce this
change, I suggest committing the above fix "as is".
+1. Looks right to me too.
Amos
