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

Reply via email to