Re: recent storeDirSelectSwapDirRoundRobin changes

Sorry for not mentioning the change. It slipped in as part of the larger
cache_dir options parsing patch.

The main reason why there is a -1 return is the "error" conditions that
may leave us without ANY possible store to use
  * read-only
  * not a acceptable object size for the configured stores
  * all stores indicate error (load < 0)

so designing it so it never ever returns -1 is quite impossible, as
there are configurations and situations where it MUST return -1. (a
stupid example is if the user reconfigures all his stores read-only)

You may ofcourse remove the "load > 1000" (overload) condition if the
goal is that round-robin should not care if the the selected store is
overloaded. overload checks is entirely optional.

/Henrik

Duane Wessels wrote:
>
> 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 Sun Mar 04 2001 - 01:22:28 MST