Dunedan Posted October 30, 2017 Report Share Posted October 30, 2017 As some of you might know I started looking into the lobby bots (EcheLOn and XpartaMuPP) a while ago with the goal to improve on some of their shortcomings. While I have nothing to share yet, I already cleaned up the code base and made it work with current versions of its dependencies and ejabberd (including mod_ipstamp), amongst several smaller improvements in various areas. There is one detail that keeps me thinking: Is there really a need for two separate bots? If my understanding of the original intention (see #3022 [PATCH] Split rating from lobby bot into another ) is correct, it was done for two reasons: 1. Allow to run the lobby without ratings functionality. 2. Remove load caused by the rating logic from the bot who's responsible to manage games. While these goals are noble, I'm not a friend of their current implementation. As currently implemented 0ad sends all lobby-related requests to XpartaMuPP. When XpartaMuPP realizes that it's something concerning ratings (rate a new game, fetch ratings to show to players, ...) it'll relay this request to EcheLOn. That relay-functionality causes a lot of complexity, makes it unnecessarily complex to fix issues like #4812 XpartaMuPP doesn't use original id for responses and causes load on XpartaMuPP which wouldn't be necessary. What I like to propose is to merge the two bots again in future and implement the following measures to fix the original issues: 1. Add a command line flag to enable/disable rating functionality. 2. Make more use of threads to distribute the load better. What are your thoughts about that? Quote Link to comment Share on other sites More sharing options...
elexis Posted October 30, 2017 Report Share Posted October 30, 2017 Having the lobby bots split in two separate processes communicating via xmpp means they could run on different computers too, so sounds like an advantage we might not necessarily want to revert. Furthermore two processes mean you can restart one of them without restarting the other, which means the gamelist is not lost if the rating bot is restarted (could be solved by saving the state to a file though). Perhaps @scythetwirler can you give you more specific feedback, as he implemented the split and has the most experience with xmpp performance. Quote Link to comment Share on other sites More sharing options...
Dunedan Posted October 30, 2017 Author Report Share Posted October 30, 2017 50 minutes ago, elexis said: Having the lobby bots split in two separate processes communicating via xmpp means they could run on different computers too Yes, that's an advantage, but is there any other reason for that than splitting the load onto two servers? 50 minutes ago, elexis said: Furthermore two processes mean you can restart one of them without restarting the other, which means the gamelist is not lost if the rating bot is restarted Does it happen that EcheLOn needs to be restarted, while XpartaMuPP doesn't? If yes, I'd be interested to know why. Quote Link to comment Share on other sites More sharing options...
leper Posted October 30, 2017 Report Share Posted October 30, 2017 One of the reasons was that ratings had some issue and that being in the single bot made the lobby unusable. Another of the possibilities that allowed was to limit the resources the rating bot could use, not that that was ever done. Merging them is a bad idea, the bots do different things and should not be one bot. Else modifying something about ratings requires taking down the main lobby functionality (match making). Using threads also will not solve anything since the bots are still in python and the GIL is still existent. What would most likely make more sense is to stop forwarding rating messages and instead send them directly to the corresponding bot. Also being able to run a lobby without ratings is something that should be supported (mostly because that requires fewer dependencies and setup that might not be wanted). 1 Quote Link to comment Share on other sites More sharing options...
elexis Posted October 30, 2017 Report Share Posted October 30, 2017 Restarting the rating bot without losing the game list, because creating smurf account is such a fun thing to do on the lobby. Quote Link to comment Share on other sites More sharing options...
elexis Posted October 30, 2017 Report Share Posted October 30, 2017 2 minutes ago, leper said: being able to run a lobby without ratings is something that should be supported (The rating GUI should become hidden in that case too some day. I guess I should speak to the simple ticket section.) Quote Link to comment Share on other sites More sharing options...
Dunedan Posted October 30, 2017 Author Report Share Posted October 30, 2017 6 minutes ago, leper said: Using threads also will not solve anything since the bots are still in python and the GIL is still existent. I seriously doubt that the GIL is the problem here, but I'd be pleased if you prove me wrong. 5 minutes ago, leper said: What would most likely make more sense is to stop forwarding rating messages and instead send them directly to the corresponding bot. That's another possible solution. Would work for me as well, as the problems I currently see are based around the relay functionality. 8 minutes ago, leper said: Also being able to run a lobby without ratings is something that should be supported […] No question, that should be supported in any case. 16 minutes ago, elexis said: Restarting the rating bot without losing the game list, because creating smurf account is such a fun thing to do on the lobby. How does that relate to restarts of EcheLOn? Isn't that simply covered by removing the accounts from the database? Quote Link to comment Share on other sites More sharing options...
leper Posted October 30, 2017 Report Share Posted October 30, 2017 Assuming there'd be multiple threads for rating updates, then the GIL itself might cause game updates to take ages, which is an issue since it impacts the main functionality. Not having different functionality in the same bot reduces the chance that a failure in one of those features takes down the lobby. 1 Quote Link to comment Share on other sites More sharing options...
elexis Posted October 30, 2017 Report Share Posted October 30, 2017 1 hour ago, Dunedan said: How does that relate to restarts of EcheLOn? Isn't that simply covered by removing the accounts from the database? If it's the cse that the sqlite db is hotloaded when edited externally, then I wasn't aware of that. Quote Link to comment Share on other sites More sharing options...
Dunedan Posted October 30, 2017 Author Report Share Posted October 30, 2017 25 minutes ago, elexis said: If it's the cse that the sqlite db is hotloaded when edited externally, then I wasn't aware of that. While SQlite claims they're able to do that (https://sqlite.org/faq.html#q5) I did did a quick test to verify that it works for EcheLOn and edited the sqlite-database file using sqlite3 while EcheLOn was running and it worked perfectly well. Suddenly my test account had a a rating of 5000. ;-) 1 Quote Link to comment Share on other sites More sharing options...
elexis Posted October 30, 2017 Report Share Posted October 30, 2017 "But only one process can be making changes to the database at any moment in time, however." (which sounds like breakage if someone rating was changed simultaenously) Quote Link to comment Share on other sites More sharing options...
Dunedan Posted December 26, 2017 Author Report Share Posted December 26, 2017 A first patch to let 0ad talk directly to EcheLOn is now available for review: https://code.wildfiregames.com/D1177 2 Quote Link to comment Share on other sites More sharing options...
Dunedan Posted January 9, 2018 Author Report Share Posted January 9, 2018 There are now a two additional patches up for review in Phabricator, whose goal it is to get "the lobby" to work with recent versions of its dependencies: D1182 – Make XMPP-bots work with newer SleekXMPP versions D1208 – Update mod_ipstamp for current ejabberd versions I'd be really glad if someone could review these patches, together with the already mentioned D1177. 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.