Add support for multiple master servers. #138
Conversation
sharifmarat
force-pushed
the
multi-master-server-conf
branch
from
f03dc9a
to
d947753
Compare
sharifmarat
force-pushed
the
multi-master-server-conf
branch
from
d947753
to
77438a2
Compare
reyandme
force-pushed
the
master
branch
2 times, most recently
from
38014ee
to
9bef3fc
Compare
| FreeAndNil(fMasterServer); | ||
| for i := Low(fMasterServers) to High(fMasterServers) do | ||
| FreeAndNil(fMasterServers[i]); | ||
| fMasterServers := nil; |
There was a problem hiding this comment.
Should be SetLength(fMasterServers, 0);
| if aMasterServerAddress[i] <> fMasterServers[i].MasterServerAddress then | ||
| recreate := true; | ||
|
|
||
| StatusMessage('(Re-)creating connections to master servers'); |
There was a problem hiding this comment.
Everything from here should only run if recreate is true?
| StatusMessage('(Re-)creating connections to master servers'); | ||
| for i := Low(fMasterServers) to High(fMasterServers) do | ||
| FreeAndNil(fMasterServers[i]); | ||
| fMasterServers := nil; |
There was a problem hiding this comment.
Delete this line: fMasterServers := nil;
|
|
||
| procedure TKMMasterServer.Error(const S: string); | ||
| begin | ||
| if Assigned(fOnError) then fOnError(S); | ||
| gLog.AddTime('Error from master server ' + GetActiveServer()); |
There was a problem hiding this comment.
Don't log it here, use fOnError to report problems and the owner can decide what to do about it
| @@ -16,19 +16,29 @@ TKMMasterServer = class | |||
| private | |||
| fIsDedicated: Boolean; | |||
|
|
|||
| fMasterServerAddress: TStringList; // List with at least one master server | |||
| fActiveMasterServer: Integer; //Currently active master server | |||
There was a problem hiding this comment.
I think it would be better if clients always query every master server and merge the results. Otherwise two friends could see different lists of servers if the main master server is unreliable but still online (e.g. not responding 50% of the time). It's not a big deal to run a few HTTP requests each time, the amount of data is small.
If you do it this way you'll need one fHTTPClient per server, and TKMServerQuery.ReceiveServerList will need to ignore duplicates.
There was a problem hiding this comment.
I agree with this point, there will be no harm if receive and adjoin replies from many masterservers, since its possbile to ignore duplicates
| @@ -11,7 +11,7 @@ TKMDedicatedServer = class | |||
| private | |||
| fLastPing, fLastAnnounce: cardinal; | |||
| fNetServer: TKMNetServer; | |||
| fMasterServer: TKMMasterServer; | |||
| fMasterServers: array of TKMMasterServer; | |||
There was a problem hiding this comment.
Since TKMMasterServer can deal with multiple servers, wouldn't it be better to have one here and let TKMMasterServer deal with each server like we do for client side? (i.e. make server/client handle it more similarly)
|
@lewinjh Thank you for detailed PR review |
reyandme
force-pushed
the
master
branch
6 times, most recently
from
e8f6208
to
7283579
Compare
reyandme
force-pushed
the
master
branch
2 times, most recently
from
8926573
to
6d43530
Compare
reyandme
force-pushed
the
master
branch
2 times, most recently
from
5f1a4ed
to
61f69db
Compare
reyandme
force-pushed
the
master
branch
2 times, most recently
from
8701bd9
to
48b05c8
Compare
Changed master setting from a single string to a list of strings
Updated Master server's query requests to re-try in case of an error from a master server. Number of re-tries is limited to number of master servers.
Dedicated server sends announcements to all master servers.