recent storeDirSelectSwapDirRoundRobin changes

 static int
-storeDirSelectSwapDirRoundRobin(const StoreEntry * unused)
+storeDirSelectSwapDirRoundRobin(const StoreEntry * e)
 {
     static int dirn = 0;
     int i;
+ int load;
     SwapDir *sd;
- /*
- * yes, the '<=' is intentional. If all dirs are full we want to
- * make sure 'dirn' advances every time this gets called, otherwise
- * we get stuck on one dir.
- */
+ ssize_t objsize = (ssize_t) objectLen(e);
     for (i = 0; i <= Config.cacheSwap.n_configured; i++) {
        if (++dirn >= Config.cacheSwap.n_configured)
            dirn = 0;
        sd = &Config.cacheSwap.swapDirs[dirn];
+ if (sd->flags.read_only)
+ continue;
        if (sd->cur_size > sd->max_size)
            continue;
+ if (!storeDirValidSwapDirSize(i, objsize))
+ continue;
+ /* check for error or overload condition */
+ load = sd->checkobj(sd, e);
+ if (load < 0 || load > 1000) {
+ continue;
+ }
        return dirn;
     }
- return dirn;
+ return -1;
 }

This change bothers me quite a bit.

First of all, there is no mention of changes to
storeDirSelectSwapDirRoundRobin in the commit messages.

I don't want storeDirSelectSwapDirRoundRobin to return -1, EVER.
And I don't think that round-robin should care about the load.
The underlying storage system can refuse to open the file if it
wants to, so for the round-robin case I think checking the "load"
is worthless.

If you don't like the way this selection algorithm works, please
don't change it. Write another one and give it a different name
like 'almost-round-robin'.

The read-only check is a good thing of course.

Duane W.
Received on Sat Mar 03 2001 - 19:29:52 MST