Ambal

Undergarun, thank you for your tests. Is your server got any bonuses from dedicated connections for SELECTs? You have so powerful PC to host MySQL server so I guess you will barely see any differences In current design for async SELECTs we utilize async connection <- this was done for simplicity. Ideally in SqlOperation class objects we need to pass pointer for SqlConnection they will use when executing queries. Currently I'm at work and cannot download images with profiling data you've posted - will take a look into them this evening. Also, in nearest future I'll definitely update a patch with configurable connection pool for SELECTs. Stay tuned

Thank you, Undergarun. I'm looking forward to get results of your tests! In case your server will be less laggish with current patch - I'll definitely implement configurable DB connection pool for SELECTs Cheers

Undergarun, can you provide us with query counters on per-database basis for characters, mangosd and realmd? It would be nice to see the full picture so we can decide how DB layer can be improved. As I said, SELECTs are synchronous in 99% of time so having connection pool for them would speed up server. For transactions it is not so simple since they are asynchronous and if they are processed by DBMS at acceptable speed - we don't need to worry about them. If not and SqlDelayThread is overflowed with transactions - this means that this particular area also needs improvement. Cheers

Undergarun, thank you greately for MySQL query counter data - it is more than helpful Currently each database has 2 (two) connections (you can see it on screenshot provided by Undergarun): 1) for SELECTs, which are executed in sync 2) for transactions, which are executed asynchronously Speaking of Database::AsyncQuery() function - in current core, unfortunately, you can successfully issue this command only from the main thread. In this patch this will be fixed. We do not use pool of connections currently since we need profiling data from MySQL to find out what is the bottleneck. Also, current core lacks 'prepared statements' which will improve DB performance even more so brute force connection spawning might not be very useful. Please note that if connection pool is not a problem for making SELECT queries - it is real PITA for making transactions. If you will see bonus from using this patch and no problems will be spotted - we can develop DB layer even more: add configurable connection pool etc. So continue doing the test and provide us with all information you can and we will use it to make mangos better and faster. P.S. Guys, currently we have alot of queries in SaveToDB() / RemoveFromDB() which are not wrapped into one single transaction. Take a look into GameObject::DeleteFromDB() - instead of one single transaction we issue up to 5(!) queries! We have more room for optimization, thats for sure. Cheers

Also, you might try following option in your MySQL config in conjunction with current patch to squeeze more performance from DBMS: [mysqld] transaction-isolation = READ-COMMITTED This will lower transaction isolation level and should speed DB queries a bit more.

Hi everyone. Here is a new patch which rewrites and refactors alot of DB code. Several features of this patch: 1) dedicated DB connections for sync (SELECT) and async (INSERT+DELETE+UPDATE) SQL queries. All transactions are executed from async connection so your DB queries will be able to run in parallel. Please, pay attention on server performance. 2) less locks are used in code - better scalability. SqlTransaction class is not using mutexes anymore. 3) allow multiple threads to issue async DB queries with callbacks. Currently only main thread was capable of doing it. 4) simplify code by making refactoring. Also here is a brief numbers about how many SQL queries of specific type we use in mangos code: 1) SELECT - 398 (191 queries are issued upon server startup: characters - 39; mangosd - 151; realmd - 1) 2) INSERT - 88 3) DELETE - 255 4) UPDATE - 158 So basically SELECTs will use their own connection, and INSERT+DELETE+UPDATE SQL queries will use the other one. This should help you boost your server performance. WARNING: please, backup your databases before starting tests!!!. In case you will spot any issues with data consistency - we will revert to using single DB connection immediately!!! Please, also test this patch with mtmaps installed. Updates: Upd#1: 'Autocommit' mode for MySQL is disabled. For atomic updates it is better to use transactions as more portable solution. Upd#2: patch is moved on rev [11010]. Upd#3: patch updated to rev [11030]. Configurable connection pool is added Up to 16 connections for SELECTs. Async queries still use dedicated connection for transactions. Also some 'SaveRespawnTime' transactions are combined into single transaction for speed. NOTE: update your mangosd.conf files! Upd#4: PostgreSQL support is added. Compilation error in prev commit is fixed. Patch is updated to rev [11040]. My repo: https://github.com/Ambal/mangos Branch: master Happy testing

Any progress on this patch?

Wooot! Lets hope problems will really go away with this change. I'm also rewriting internals of our DB code to make it more simple, reliable, utilize less locks + add some performance by using dedicated DB connections for queries and transactions. Cheers

Did anyone of you managed to test branch 1 of this patch?

cyberium, you should check Group::SendUpdate() code then - it contains ObjectAccessor::FindPlayer() calls which produced issues in HandleRequestPartyMemberStatsOpcode(). EDIT: replacing ObjectAccessor::FindPlayer() by HashMapHolder<Player>::Find() function calls in Group::SendUpdate() should solve your issue.

EDIT: in current mangos core this is a no-go since GetZoneId() function in HandleRequestPartyMemberStatsOpcode() will call GetMap() function for player which is not in world -> ASSERT will fail. Not applicable.

Can you provide compilation error logs? W/o them we are unable to resolve your issue.

We do use async SQL requests - they are mostly for INSERT statements. Also we already have a subsystem which is capable to post and process SQL query completion callbacks to the main thread. Some words about several connections to DB - it is not so difficult to do, but this is not a straight-forward solution since ALL SQL requests from separate object must be made on dedicated SQL connection otherwise there might be BIG problems with data consistency even if we use transactions. Current design with single DB connection does not have this issue because there are no concurrent queries. We can think of redesigning DB layer since it lacks some performance features like 'prepared statements', but this patch will be HUGE and will take alot of time to code and test. P.S. At current stage 'prepared statements' will give us more performance bonuses than several DB connections. So my bets on this functionality. There was a patch made by Wyk3d loooooong ago but it didn't get into main repo, unfortunately. Cheers

cyberium, if you are unsure if your packet handler is safe to run in Map::Update() - don't touch it. Also, if your packet handler is light-weight e.g. no heavy vmap/mmaps logic being called or amount of such packets is low - don't bother making it thread-safe either. Cheers

I agree with qsa - we already implement 'async IO' by using thread pool doing durty work with synchronous sockets. Thats why, unfortunately, only tests can tell us if the gain will be noticable or not Anyway, personal cheers for your attempt to bring true async networking IO to mangos P.S. IMO, for networking code we already have acceptable level of performance. What do we need is stability since crashes in network code are really weird and annoying

It is really better to add another column to 'spell_threat' table since it is not 'separate functionality'

Patch applied in [10924]. If you'll find issues - report them ASAP. Thanks to everyone involved

Yes, you are right, Undergarun: we will try to put as much thread-safe code processing into Map::Update() as possible. All thread-unsafe or complex functionality will be processed in the main thread. Also, if mangos will become "multimangos" - all outer-world communications/requests will be processed as WorldTasks (login, logout, chat, map transfers etc). As of reported crashes in networking code, I agree with qsa - this issue is not very easy to reproduce. So try to increase output buffer for packets maybe twice and report if your server stability has increased or not.

If you dislike Windows, why ask? Answer to your question: depends on amount of players which will play on your server. For servers with online < 800-1000 you can easily use Windows.

Update for Boost::shared_ptr<> - currently it does not maintain the list of all places where protected object is referenced. So you can use this implementation w/o any worries about performance. Just be sure that you use Boost version >= 1.33.0 - recent versions make use of atomic operations and are much faster to previous revisions.

Once I suggested to use scoped pointers "auto_ptr<>" in DB code to manage SQL query results - this way we could get rid of ALL memory leaks caused by un-released resultsets, but Vladimir said that these pointers will degrade performance ( which was very funny argument since auto_ptr<> is completely inlined in release builds ). Be very careful using shared_ptr<> - try to pass these objects by reference as an input function parameters since reference counting is expensive operation. Also note, Boost::shared_ptr<> is a VERY heavy-weight solution . It should be used in only three cases: 1) you are unable to find any other thread-safe shared_ptr solution 2) performance is not your major concern 3) you need to be able to control object lifetime, e.g. destroy the object and reset ALL pointers to the same object to NULL by calling boost::shared_ptr::reset() function - might be extremely helpful functionality but it is very expensive operation. As an example: a) you create a mob b) your mob is referenced in thread manager, in attacker list, as aura target etc.. c) your mob dies and you need to remove this object safely from the world. d) you call shared_ptr::reset() function and ALL pointers to this mob are set to NULL. No crashes, no daggling pointers, no memory leaks - wooohooo

I'll take a look what went wrong with BG teleports. Selector, are you able to teleport into Arena with .tele command?

1) Threading Building Blocks (TBB) library itself needs to be upgraded in mangos - it is almost a year as outdated and lacks tonns of performance and bug fixes. 2) Using the way of doing spell updates as in the Kero99 patch is not very efficient because it requires using global mutex, which can lag your server to the death... I'm preparing another solution "World Tasks" which will allow you to execute all complex and/or thread-unsafe functions in main thread w/o overcomplicating mangos code with locks etc. As an example, player far teleports are already using it in my repo, branch "world_tasks", and server will now not try to teleport player into dungeon if it is impossible. Stay tuned

The only problem using smart pointers in mangos is Vladimir http://www.boost.org/doc/libs/1_43_0/libs/smart_ptr/sp_techniques.html - this document from Boost should help you understand how to utilize shared_ptr<> effectively. P.S. Also take a look into this document from Boost about thread-safety of shared_ptr<>: http://www.boost.org/doc/libs/1_37_0/libs/smart_ptr/shared_ptr.htm#ThreadSafety

Merry Christmas to everyone