Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/161#discussion_r120224307
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/RestrictedGuacamoleTunnelService.java
---
@@ -180,8 +187,8 @@ protected ModeledConnection
acquire(RemoteAuthenticatedUser user,
@Override
public int compare(ModeledConnection a, ModeledConnection b) {
- return getActiveConnections(a).size()
- - getActiveConnections(b).size();
+ return ((getActiveConnections(a).size() + 1) *
b.getConnectionWeight() -
--- End diff --
> How much does it actually mess up the math? Since basically all we're
doing is sorting the array based on the sign and an integer value, does it
actually mess up the calculation enough to even throw off the sorting
Quite a bit, actually.
With the `+ 1`, the calculations effectively assume that each server has
one more connection than they actually do. Depending on the weights involved,
that one extra connection can unbalance things considerably:
Take a look at the 21 connection case above (row 24). Without the `+ 1`,
the number of connections allocated to A and B stays around the expected 10:1
ratio... but *with* the `+ 1`, the ratio gets all the way up to 20:1 before
things are rebalanced properly.
> Is it a lose-lose situation? I'm trying to think of another way to run
the calculation so as to account for zero connections but not throw off the
math, and nothing is come to mind. Is it simply a case where we have to choose
one path or the other, and accept that one of the cases is going to be flawed?
The only way would be to compare the weights as a means of breaking ties.
That way, servers are always ordered based on a correct calculation of their
relative load, and a sensible choice is made in the case that each server is
under equivalent load.
> If there isn't a way to resolve it so that both cases work, any
preferences on which one to stick with the win on? I would assume we'd want to
ignore the 0 connection case in favor of the other ones being correct (if it
matters enough to worry about it), but any reason we'd want 0 to be correct
over lower connection counts?
The one that's actually correct. ;)
In this case, though, it seems we can have it both ways - we just need to
make sure the math is sound.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
