multi-wms servers should use deterministic server id

[crschmidt]

Non-deterministic server choice is a bad idea. When I view a tile, I may get caching headers with that URL from a properly configured server. By randomly changing the server I request from, the browser has to store up to numUrls caches for each tile.

Instead, make this deterministic based on the integer value of the request parameters, modded into the severs array. Deployed on  http://labs.metacarta.com/wms-c/demo.html, patch incoming against SVN.

[crschmidt]

Patch attached, please review.

[crschmidt]

Newest patch includes tests.

[crschmidt]

I'm not sure I'm happy with the level of randomness this provides. Don't review/commit yet. I'll see if I can come up with some better way of doing this.

[tschaub]

alternative deterministic server indexing for multi-wms

[tschaub]

The multi-wms.patch is a quick hack at an alternative way to determine a server index. This would need to be done for each layer type that allowed multiple servers. It is also seems horribly inefficient and probably not ready for serious review - just a thought.

[sderle]

deterministic server selection based on simple real number hashing algorithm

[sderle]

Latest patch provides deterministic server selection for WMS, TMS, ka-Map via HTTPRequest. Slight changes are needed to individual layers because those layers contain the information on tile bounds that's used to hash an index into the layer.url array. We could conceivably apply said changes to other layers as well (MapServer, WMS.Untiled) but I figured it belonged in these layers first. Tests are included in the patch.

Please review and apply. All tests should pass.

[tschaub]

bounds.minx and bounds.miny should be changed to bounds.left and bounds.bottom in the WMS.js part of your patch (though I like the more explicit better)

[tschaub]

same as above but with left,bottom instead of minx,miny

[sderle]

this is great but it needs a fall back for layers that don't call selectUrl

[sderle]

merger of my and Chris's patch

[crschmidt]

I'm happy with it. Commit it.

[sderle]

fixed by r2821.

[euzuro]

This patch breaks tests in both IE and Opera. And for a good reason. Patch forthcoming.

[euzuro]

this fixes the httplayer in opera and ie, and does not break in ff either. the idea is that you cannot safely pass the url variable to Util.getArgs() anymore now that it could be either a string or an array of urls (which, of course, is a perfect example of why making a variable like that which can be of two different types is generally not a good idea). So what we do is we do the deterministic url selecting bit *before* that call... and then we rebuild the paramString again after the call and the check. I have added some basic coments to at least correctly document the type of the 'url' property

[euzuro]

I would like to note that before this ticket should be closed as 'fixed' again, it would sure be nice if someone could take the three minutes necessary to put some comments in the seletctUrl function.

I do not consider myself the wisest of the wise, yet neither am i the most novice programmer or OL user, and I can tell you that I had *no* idea what is going on in this function at first glance. I had to do an svn blame to see when it was added, then read up on the ticket for more info.

I see this comment, inline lower in the code, where the function gets called:

    // in which case we will deterministically select one of them in 
    // order to evenly distribute requests to different urls.

This is nice but we should see that explanation (namely, the 'deterministically' bit) up in the function comment as well.

To me, any time you are doing wierd math like:

(Math.sqrt(5) - 1) / Math.sqrt(3)

... you should probably have at least a loose explanation of what is going on there.

(note that this value, by the way, should be replaced by a constant so that it's not getting re-computed for each character in the paramstring ((and so that the line doesnt go over the 79 character limit)) )

[sderle]

patch incorporating Erik's last patch plus recommended changes

[euzuro]

fixed with r2852. only added small comments bit, tests fix from sde's final patch. all is groovy.