Tunnel autochooser #179
Tunnel autochooser #179
Conversation
* better load balancing for tunnels by selecting slightly higher ping tunnel when it has fewer users * set maxClients limit to 600 so we can do fine tuning via master-list * never select full servers * Move rating to CnCNetTunnel class * Try to use best tunnel on failure * auto-switch tunnel if it's full
…server after each ping.
There was a problem hiding this comment.
Tested it, this algorithm finally gave me adequate tunnel server with low ping. Overall the changes seem good but lack explanation in some parts of them, and the magic numbers need to be converted to constants and I suggest to use some kind of a quadratic function to set a rating for a server. I might take a stab at doing it myself though.
| get | ||
| { | ||
| if (Clients + 24 >= MaxClients) | ||
| return int.MaxValue; |
There was a problem hiding this comment.
In my opinion it would be a lot better to use something like a quadratic or exponential function to calculate rating than hardcap at MaxClients - 24. Also, magic numbers antipattern.
There was a problem hiding this comment.
I think a more complicated algorithm might be better in narrow cases but IMO the complexity out weighs the benefit.
There was a problem hiding this comment.
I've already written a proto. algorithm, it isn't so hard, just needs adjustments to be done.
There was a problem hiding this comment.
The point of this code is not to select tunnels with fewer clients (that's handled elsewhere). The point of this code comes from the fact that the tunnel chooser first runs when the game lobby is created and after each ping, there is no way to know if the selected tunnel will have enough resources to host our game when we're ready to start (ie, resource is requested/granted on game start). So we're creating some headroom that will give us a reasonable chance to get our first tunnel choice on game start.
| if (playerPorts.Count < Players.Count && tunnelHandler.Tunnels.Any()) | ||
| { | ||
| tunnelHandler.CurrentTunnel = tunnelHandler.Tunnels.Aggregate((i1, i2) => i1.Rating < i2.Rating ? i1 : i2); | ||
| playerPorts = tunnelHandler.CurrentTunnel.GetPlayerPortInfo(Players.Count); |
There was a problem hiding this comment.
I'm against hidden changes of tunnel server, and this stuff also makes the whole following codeblock obsolete.
There was a problem hiding this comment.
I've added logging here so that the change is no longer hidden.
There was a problem hiding this comment.
Perhaps I didn't phrase it clear enough, my bad. I meant that the user should be aware of the tunnel being chosen at least. Also the next codeblock after that still duplicates this one's functionally and won't be used in 99% cases.
The best possible solution I see is to just select the suggested tunnel server in the opened window if the previous tunnel was manually selected, and in case the previous tunnel was automatically selected add a notice to chatbox that the tunnel was changed to TunnelName instead.
There was a problem hiding this comment.
This bit of code is designed to reduce frustration when a player wants to start a game and the tunnel server is out of resources. Manual tunnel selection is an advanced option now but it will be exclusively a power user option in the future when v3 tunnels are implemented.
The manual selection code isn't really ready to handle corner cases like this. It's a bit hacky, it should have it's own PR to handle it correctly. I don't think it's a good idea to block this PR for that reason.
There was a problem hiding this comment.
The code redundancy must be fixed either way, preferably in a way I proposed - change tunnel server silently only in case the user didn't select the tunnel manually before.
PR shouldn't introduce more inoptimal solutions code-wise, in my opinion.
…hm and simplifies it by using integer math.
| if (playerPorts.Count < Players.Count && tunnelHandler.Tunnels.Any()) | ||
| { | ||
| tunnelHandler.CurrentTunnel = tunnelHandler.Tunnels.Aggregate((i1, i2) => i1.Rating < i2.Rating ? i1 : i2); | ||
| playerPorts = tunnelHandler.CurrentTunnel.GetPlayerPortInfo(Players.Count); |
There was a problem hiding this comment.
Perhaps I didn't phrase it clear enough, my bad. I meant that the user should be aware of the tunnel being chosen at least. Also the next codeblock after that still duplicates this one's functionally and won't be used in 99% cases.
The best possible solution I see is to just select the suggested tunnel server in the opened window if the previous tunnel was manually selected, and in case the previous tunnel was automatically selected add a notice to chatbox that the tunnel was changed to TunnelName instead.
| /// Rating Factor: | ||
| /// The amount of free client slots needed for the tunnel to be eligible for autoselection | ||
| /// </summary> | ||
| private const int CLIENTS_HEADROOM = 24; |
There was a problem hiding this comment.
Why don't use some sort of a percentage instead? Hardcaps are bad in this case, imagine a tunnel with 50 players max. being filled with 26 players and stopping receiving players.
There was a problem hiding this comment.
Live example: Berryville tunnel has 50 slots, so the server will be running at 26 players max. in case of autochoosing it?
I think the best would be to use tangent function here, so when the tunnel clients amount is very close to max - the rating will skyrocket thus making it never chosen automatically.
There was a problem hiding this comment.
The CLIENTS_HEADROOM is not for performance reasons. It's meant to make sure that when we go to use a tunnel server it has enough free slots to fill our request. The problem occurs because there is a time difference between when we select the tunnel server (game lobby creation) to when we request the user slots (game start).
There was a problem hiding this comment.
I understood that already, yet the issue I mentioned is still here. Servers with low max. player amounts would never be chosen or chosen rarely.
IMO the issue of time difference should be dealt with some other sort of measure, f.ex. a task in game lobby checking for availability of the current tunnel on tunnel refresh. Ideally the players in lobby would be counted as players on servers even before starting but that would require server-side changes.
| get | ||
| { | ||
| if (Clients + 24 >= MaxClients) | ||
| return int.MaxValue; |
There was a problem hiding this comment.
I've already written a proto. algorithm, it isn't so hard, just needs adjustments to be done.
| } | ||
| else | ||
| { | ||
| return UNTRUSTED_RATING + Clients; |
There was a problem hiding this comment.
With this code it looks like the clients would almost never get an unofficial and non-recommended server selected. This is not the behavior I see as good, CnCNet is meant to be a community service, and IMO recommendation should boost the server, not basically "whitelist" recommended servers.
There was a problem hiding this comment.
This is the best case, the recommended servers have all been tested and proven to be good. The goal is for the users to never want or need to select a tunnel server themselves. When they do manually select a tunnel server it should be their own server that they want to use for personal reasons. This provides the added benefit that we can do some hacks on the master-list side to balance tunnel servers during the high load times.
There was a problem hiding this comment.
I still object against verified and official tunnels being effectively whitelisted against regular ones, this is not future-proof at all. I mean the example is already at hand. I have a feeling no one actually bothered to test Berryville, yet it has very low ping for Europe and I never had issues using it. Imagine CnCNet staff going very busy all at once, then some of the verified tunnel will also go off and the players will be automatically balanced to 2-3 verified servers in USA, for example, even if they are from EU and there is regular, non-verified EU servers. This is absolutely not a desirable outcome IMO.
There was a problem hiding this comment.
The reason we should never use untrusted tunnel servers is because they fail more often. When a tunnel server fails and it breaks the game for a group of players, the players don't blame the tunnel server owner they blame CnCNet, we can't stake our reputation on untrusted servers.
Imagine CnCNet staff going very busy all at once, then some of the verified tunnel will also go off and the players will be automatically balanced to 2-3 verified servers in USA, for example, even if they are from EU and there is regular, non-verified EU servers. This is absolutely not a desirable outcome IMO.
That's still more desirable than a broken tunnel server with low ping destroying everyone's game.
There was a problem hiding this comment.
My view of future CnCNet is as of a community service, not an organization, so I don't think this outcome is more desirable. I don't say that the system should be ditched. Per my idea verified servers would be "boosted" and chosen most of the time, but in case there's no local verified servers with low enough ping - regular one would be chosen. This would never happen in the current implementation.
Again, you say that untrusted servers are bad because all good servers are made verified. Is that so? Not really. Someone has to test them manually, and as I said I don't see anyone testing Berryville server or some other EU servers which are OK despite the lack of EU tunnels. I always get Canada East chosen and I live in Europe.
| if (playerPorts.Count < Players.Count && tunnelHandler.Tunnels.Any()) | ||
| { | ||
| tunnelHandler.CurrentTunnel = tunnelHandler.Tunnels.Aggregate((i1, i2) => i1.Rating < i2.Rating ? i1 : i2); | ||
| playerPorts = tunnelHandler.CurrentTunnel.GetPlayerPortInfo(Players.Count); |
There was a problem hiding this comment.
The code redundancy must be fixed either way, preferably in a way I proposed - change tunnel server silently only in case the user didn't select the tunnel manually before.
PR shouldn't introduce more inoptimal solutions code-wise, in my opinion.
| /// Rating Factor: | ||
| /// The amount of free client slots needed for the tunnel to be eligible for autoselection | ||
| /// </summary> | ||
| private const int CLIENTS_HEADROOM = 24; |
There was a problem hiding this comment.
Live example: Berryville tunnel has 50 slots, so the server will be running at 26 players max. in case of autochoosing it?
I think the best would be to use tangent function here, so when the tunnel clients amount is very close to max - the rating will skyrocket thus making it never chosen automatically.
| } | ||
| else | ||
| { | ||
| return UNTRUSTED_RATING + Clients; |
There was a problem hiding this comment.
I still object against verified and official tunnels being effectively whitelisted against regular ones, this is not future-proof at all. I mean the example is already at hand. I have a feeling no one actually bothered to test Berryville, yet it has very low ping for Europe and I never had issues using it. Imagine CnCNet staff going very busy all at once, then some of the verified tunnel will also go off and the players will be automatically balanced to 2-3 verified servers in USA, for example, even if they are from EU and there is regular, non-verified EU servers. This is absolutely not a desirable outcome IMO.
Adds the new tunnel selection algorithm that's designed to distribute players over tunnels more efficiently.