Fixed entry size calculation for the max-size cache_dir selection
algorithms.
There were two sides of this bug:
In src/store_swapout.cc, we must create metadata earlier because
storeCreate() needs swap_hdr_sz. With swap_hdr_sz unknown at the time of
storeCreate(), the SwapDir selection algorithms may select SwapDirs that
should not really take the entry as the real storage size (with the
metadata swap_hdr_sz) would exceed the store slot size.
In src/store_dir.cc, we must add the metadata size before looking for
cache_dirs that accept objsize. Only the "new"
storeDirSelectSwapDirRoundRobin selection scheme was affected.
HTH,
Alex.
P.S. I do not have a version of this patch generated against trunk, but
I hope it applies easily.
Fixed entry size calculation for the max-size cache_dir selection algorithms.
There were two sides of this bug.
In src/store_swapout.cc, we must create metadata earlier because storeCreate()
needs swap_hdr_sz. With swap_hdr_sz unknown at the time of storeCreate(), the
SwapDir selection algorithms may select SwapDirs that should not really take
the entry as the real storage size (with the metadata swap_hdr_sz) would
exceed the store slot size.
In src/store_dir.cc, we must add the metadata size before looking for
cache_dirs that accept objsize. Only the "new" storeDirSelectSwapDirRoundRobin
selection scheme was affected.
=== modified file 'src/store_dir.cc'
--- src/store_dir.cc 2009-05-30 13:38:39 +0000
+++ src/store_dir.cc 2009-09-02 04:59:32 +0000
@@ -189,6 +189,10 @@
int load;
RefCount<SwapDir> sd;
+ ssize_t objsize = e->objectLen();
+ if (objsize != -1)
+ objsize += e->mem_obj->swap_hdr_sz;
+
for (i = 0; i <= Config.cacheSwap.n_configured; i++) {
if (++dirn >= Config.cacheSwap.n_configured)
dirn = 0;
@@ -201,7 +205,7 @@
if (sd->cur_size > sd->max_size)
continue;
- if (!sd->objectSizeIsAcceptable(e->objectLen()))
+ if (!sd->objectSizeIsAcceptable(objsize))
continue;
/* check for error or overload condition */
=== modified file 'src/store_swapout.cc'
--- src/store_swapout.cc 2009-02-01 10:09:23 +0000
+++ src/store_swapout.cc 2009-09-02 05:23:45 +0000
@@ -63,6 +63,15 @@
/* If we start swapping out objects with OutOfBand Metadata,
* then this code needs changing
*/
+
+ /* TODO: make some sort of data,size refcounted immutable buffer
+ * and stop fooling ourselves with "const char*" buffers.
+ */
+
+ // Create metadata now, possibly in vain: storeCreate needs swap_hdr_sz.
+ const char *buf = e->getSerialisedMetaData ();
+ assert(buf);
+
/* Create the swap file */
generic_cbdata *c = new generic_cbdata(e);
sio = storeCreate(e, storeSwapOutFileNotify, storeSwapOutFileClosed, c);
@@ -70,6 +79,7 @@
if (sio == NULL) {
e->swap_status = SWAPOUT_NONE;
delete c;
+ xfree((char*)buf);
storeLog(STORE_LOG_SWAPOUTFAIL, e);
return;
}
@@ -85,16 +95,6 @@
e->swap_dirn = mem->swapout.sio->swap_dirn;
/* write out the swap metadata */
- /* TODO: make some sort of data,size refcounted immutable buffer
- * for use by this sort of function.
- */
- char const *buf = e->getSerialisedMetaData ();
-
- /* If we start swapping out with out of band metadata, this assert
- * will catch it - this code needs to be adjusted if that happens
- */
- assert (buf);
-
storeIOWrite(mem->swapout.sio, buf, mem->swap_hdr_sz, 0, xfree);
}