lör 2006-08-05 klockan 15:45 +0800 skrev Steven: > if (fd >= 0 && mem->inmem_hi - mem->inmem_lo > SM_PAGE_SIZE + > Config.Store.maxInMemObjSize + Config.readAheadGap) { > - storeDeferRead(e, fd); > - return 1; > + if(storeLowestMemReaderOffset(e) != mem->inmem_hi) { > + storeDeferRead(e, fd); > + return 1; > + } > }
Hmm.. first I thought this this effectively disables the whole block, but maybe you are right. Lets see. The purpose of this block is: /* Just a small safety cap to defer storing more data into the object * if there already is way too much. This handles the case when there * is disk clients pending on a too large object being fetched and a * few other corner cases. */ under these conditions storeLowestMemReaderOffset returns what? Well, it depends. If there is no clients at all it returns inmem_hi + 1. If there is no other clients it returns inmem_hi + 1. If the memory client is still around it returns the offset of the memory client.. so no, I don't think the above will work out (even if fixed for the +1). It both allows memory usage to explode and still allows for the stall to happen making the code even further unpredictable.. It's then better to keep it as it is and look into why data didn't get freed, or delete the block and accepting that memory usage may explode under these conditions.. Note: As long as we allow multiple clients on the same incomplete object we will run into cornercases like this, and it's far from obvious how to handle them all.. In the specific case you are seeing the correct fix is to look into why memory had not been freed. It should have been freed. The condition above is there to avoid blowing up memory usage outside any proportions which will very negatively impact everything, not just this request so disabling it is not a good option. Regards Henrik
signature.asc
Description: Detta är en digitalt signerad meddelandedel