Re: A little help with cbdata and requestLink
Mark Nottingham wrote: Would someone mind taking a quick look at this patch: http://www.squid-cache.org/bugs/attachment.cgi?id=1989 and telling me if I've royally stuffed up with managing fwdState and request linking? It's to make miss_access a slow lookup... Looks okay to these uneducated eyes. Probably best to wait for someone else to double-check before a HEAD commit, but IMO it looks good enough for a patching. This one is long-awaited by many. Thanks. Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1
Re: A little help with cbdata and requestLink
Thanks. Anybody else have a second to look? On 11/06/2009, at 11:28 PM, Amos Jeffries wrote: Mark Nottingham wrote: Would someone mind taking a quick look at this patch: http://www.squid-cache.org/bugs/attachment.cgi?id=1989 and telling me if I've royally stuffed up with managing fwdState and request linking? It's to make miss_access a slow lookup... Looks okay to these uneducated eyes. Probably best to wait for someone else to double-check before a HEAD commit, but IMO it looks good enough for a patching. This one is long-awaited by many. Thanks. Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1 -- Mark Nottingham m...@yahoo-inc.com
Re: A little help with cbdata and requestLink
On 06/16/2009 09:57 PM, Mark Nottingham wrote: > Thanks. Anybody else have a second to look? Please s/fwdStartFoo/fwdContinue/ and document what it is. Since this is Squid2 you do not have to do it, of course. Your cbdata and request manipulations appear technically correct to me. IMHO, the temporary lack of requestLink is poor style that will be dangerous for future code modifications. Cheers, Alex. > On 11/06/2009, at 11:28 PM, Amos Jeffries wrote: > >> Mark Nottingham wrote: >>> Would someone mind taking a quick look at this patch: >>> http://www.squid-cache.org/bugs/attachment.cgi?id=1989 >>> and telling me if I've royally stuffed up with managing fwdState and >>> request linking? >>> It's to make miss_access a slow lookup... >> >> Looks okay to these uneducated eyes. Probably best to wait for >> someone else to double-check before a HEAD commit, but IMO it looks >> good enough for a patching. >> >> This one is long-awaited by many. Thanks. >> >> Amos >> -- >> Please be using >> Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 >> Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1 > > -- > Mark Nottingham m...@yahoo-inc.com >
Re: A little help with cbdata and requestLink
OK. WRT requestlink, I was unsure what would happen if the request didn't go forward; will moving requestLink at the bottom up to here: fwdState->request = r; /* requestLink? */ do it? Thanks, On 27/06/2009, at 3:05 AM, Alex Rousskov wrote: On 06/16/2009 09:57 PM, Mark Nottingham wrote: Thanks. Anybody else have a second to look? Please s/fwdStartFoo/fwdContinue/ and document what it is. Since this is Squid2 you do not have to do it, of course. Your cbdata and request manipulations appear technically correct to me. IMHO, the temporary lack of requestLink is poor style that will be dangerous for future code modifications. Cheers, Alex. On 11/06/2009, at 11:28 PM, Amos Jeffries wrote: Mark Nottingham wrote: Would someone mind taking a quick look at this patch: http://www.squid-cache.org/bugs/attachment.cgi?id=1989 and telling me if I've royally stuffed up with managing fwdState and request linking? It's to make miss_access a slow lookup... Looks okay to these uneducated eyes. Probably best to wait for someone else to double-check before a HEAD commit, but IMO it looks good enough for a patching. This one is long-awaited by many. Thanks. Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1 -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Re: A little help with cbdata and requestLink
fre 2009-06-26 klockan 11:05 -0600 skrev Alex Rousskov: > On 06/16/2009 09:57 PM, Mark Nottingham wrote: > > Thanks. Anybody else have a second to look? > > Please s/fwdStartFoo/fwdContinue/ and document what it is. Since this is > Squid2 you do not have to do it, of course. > > Your cbdata and request manipulations appear technically correct to me. > IMHO, the temporary lack of requestLink is poor style that will be > dangerous for future code modifications. Agreed on all points. In addition i would like the internal & cacheobj checks to be moved up before miss_access, but that is a separate change. Regards Henrik
Re: A little help with cbdata and requestLink
On 06/29/2009 12:30 AM, Mark Nottingham wrote: > OK. > > WRT requestlink, I was unsure what would happen if the request didn't go > forward; will moving requestLink at the bottom up to here: > fwdState->request = r; /* requestLink? */ > do it? Yes, provided you adjust the code to call "unlink" if the request did go forward but not all the way through to where fwdState currently unlinks. Alex. > On 27/06/2009, at 3:05 AM, Alex Rousskov wrote: > >> On 06/16/2009 09:57 PM, Mark Nottingham wrote: >>> Thanks. Anybody else have a second to look? >> >> Please s/fwdStartFoo/fwdContinue/ and document what it is. Since this is >> Squid2 you do not have to do it, of course. >> >> Your cbdata and request manipulations appear technically correct to me. >> IMHO, the temporary lack of requestLink is poor style that will be >> dangerous for future code modifications. >> >> Cheers, >> >> Alex. >> >> >> >>> On 11/06/2009, at 11:28 PM, Amos Jeffries wrote: >>> Mark Nottingham wrote: > Would someone mind taking a quick look at this patch: > http://www.squid-cache.org/bugs/attachment.cgi?id=1989 > and telling me if I've royally stuffed up with managing fwdState and > request linking? > It's to make miss_access a slow lookup... Looks okay to these uneducated eyes. Probably best to wait for someone else to double-check before a HEAD commit, but IMO it looks good enough for a patching. This one is long-awaited by many. Thanks. Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1 >>> >>> -- >>> Mark Nottingham m...@yahoo-inc.com >>> >> > > -- > Mark Nottingham m...@yahoo-inc.com >
Re: A little help with cbdata and requestLink
If I parse that correctly, I think there are no such cases; i.e., all of the paths where fwdContinue doesn't go to peerSelect don't go forward, they just error out. E.g., if I read you correctly, it's not necessary to unlink the request here: case PROTO_INTERNAL: cbdataFree(fwdState); internalStart(r, e); return; correct? See modified patch below. miss_access_slow.patch Description: Binary data Thanks again, On 29/06/2009, at 11:12 AM, Alex Rousskov wrote: On 06/29/2009 12:30 AM, Mark Nottingham wrote: OK. WRT requestlink, I was unsure what would happen if the request didn't go forward; will moving requestLink at the bottom up to here: fwdState->request = r; /* requestLink? */ do it? Yes, provided you adjust the code to call "unlink" if the request did go forward but not all the way through to where fwdState currently unlinks. Alex. On 27/06/2009, at 3:05 AM, Alex Rousskov wrote: On 06/16/2009 09:57 PM, Mark Nottingham wrote: Thanks. Anybody else have a second to look? Please s/fwdStartFoo/fwdContinue/ and document what it is. Since this is Squid2 you do not have to do it, of course. Your cbdata and request manipulations appear technically correct to me. IMHO, the temporary lack of requestLink is poor style that will be dangerous for future code modifications. Cheers, Alex. On 11/06/2009, at 11:28 PM, Amos Jeffries wrote: Mark Nottingham wrote: Would someone mind taking a quick look at this patch: http://www.squid-cache.org/bugs/attachment.cgi?id=1989 and telling me if I've royally stuffed up with managing fwdState and request linking? It's to make miss_access a slow lookup... Looks okay to these uneducated eyes. Probably best to wait for someone else to double-check before a HEAD commit, but IMO it looks good enough for a patching. This one is long-awaited by many. Thanks. Amos -- Please be using Current Stable Squid 2.7.STABLE6 or 3.0.STABLE15 Current Beta Squid 3.1.0.8 or 3.0.STABLE16-RC1 -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com