Re: [PREVIEW] Making Store work with SMP-shared cache_dirs

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Mon, 16 May 2011 21:36:51 +1200

On 16/05/11 07:47, Alex Rousskov wrote:
> On 02/15/2011 01:24 AM, Amos Jeffries wrote:
>> On 15/02/11 17:41, Alex Rousskov wrote:
>
>>> The same method is used for cache_dir load reporting (if it returns
>>> true). Load management needs more work, but the current code is no worse
>>> than the old one in this aspect, and further improvements are outside
>>> this change scope.
>>
>> In UFSSwapDir::canStore I think load needs to be set regardless of the
>> shedLoad() results. If I understand it right that would help balance
>> things better when they are all overloaded.
>
> It is possible that reporting cache_dir load regardless of object
> acceptability would be better, but the way the current/old code is
> written, it would not make a difference because if the object cannot be
> stored by cache_dir, the cache_dir load is ignored: We simply switch to
> the next cache_dir. And in storeDirSelectSwapDirRoundRobin() we did not
> even compute the load until the object was accepted.
>
> We preserved that logic because (a) it kind of make sense and (b)
> changing/optimizing it would be outside this work scope.
>
>
>>> * When swapout initiation fails, release StoreEntry. This prevents the
>>> caller code from trying to swap out again and again because swap_status
>>> becomes SWAPOUT_NONE.
>>>
>>> TODO: Consider add SWAPOUT_ERROR, STORE_ERROR, and similar states. It
>>> may solve several problems where the code sees _NONE or _OK and thinks
>>> everything is peachy when in fact there was an error.
>>>
>>
>> in StoreEntry::swapoutPossible the logic tests of (currentEnd>
>> store_maxobjsize) is broken. To reach it we already have checked
>> (expectedEnd> 0 and expectedEnd< store_maxobjsize) so if (expectedEnd
>> < currentEnd) there is a bug somewhere.
>
> I see what you are saying, but I am not 100% sure that endOffset() may
> not exceed expectedReplySize() for range responses with known
> Content-Length. If you are correct, then the currentEnd check should be
> indeed removed, but leaving it should not break things either.
>
> Do you remember whether endOffset() may be very large for, say, a
> "last-byte" Content-Range response when the underlying entity is huge?
>

I'm not aware what is done for ranges.

As I mentioned in audit of the part 1 patch from today. Yes the
currentEnd check should remain. It is needed for when endOffset() == -1.
Just that it is down a bit too far in the order of checks.
  An unknown-length objects with currentEnd>max bytes will be left
marked as "uncertain" in the current patches due to the truth of
(endOffset == -1) < store_maxobjsize [&& store_maxobjsize >= 0]

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.12
   Beta testers wanted for 3.2.0.7 and 3.1.12.1
Received on Mon May 16 2011 - 09:37:07 MDT

This archive was generated by hypermail 2.2.0 : Wed May 18 2011 - 12:00:53 MDT