Projects tigase _server server-core Issues #1254
Slow startup and shutdown (#1254)
Closed
Andrzej Wójcik (Tigase) opened 3 years ago

It takes a while to start Tigase and to stop it. It would be good to improve those times as if stopping is not done in 20s JVM is killed and not all termination handlers may be called.

Andrzej Wójcik (Tigase) commented 3 years ago

I've reviewed startup and shutdown times of Tigase XMPP Server and make some changes to improve them.

  1. Switch to the new JDBC driver We are bundling Tigase with the new driver but we were using an older one (not update the reference to a class of a driver). That created warnings in logs and could result in using older version of a driver.

  2. Shutdown hooks never finished Due to the use of ThreadGroup they were never completed as during execution of shutdown MySQL driver could (and did) create a new thread for SQL execution timeout timer, which resulted ThreadGroup having one more thread which was always alive and kept Tigase from shutting down (due to how completion of shutdown hooks was checked). New code fixes this issue as it checks only threads that were created for the shutdown hooks.

  3. Aggregation of threads for shutdown in AbstractMessageReceiver Due to a lot of threads being closed in batches and each batch waiting up to 100ms for termination we were losing time on shutdown. I've reduced sleep time to 10ms and aggregated all threads in a single batch to reduce this delay.

  4. Workaround for prepareStatement() being slow on MySQL If a lot of connections are used, we have to create prepareStatement() for all procedures. For most database we were using { call TIG_**() } and prepareCall() method of the SQL driver. I've found a faster approach for MySQL with use of prepareStatement() and CALL TIG_**() instead as it doesn't require communication with MySQL server, while previous approach caused a delay when we waited for MySQL which could queue all our requests. This is workaround is disabled by can be enabled by setting 'useCallableMysqlWorkaround' = true in proper data source block.

Andrzej Wójcik (Tigase) commented 3 years ago

All changes are in issue #1254 branch.,

wojciech.kapcia@tigase.net commented 3 years ago

Regarding (3)

https://github.com/tigase/tigase-server/commit/eac5f5fe9a928972e8b7c46f55a6d1127a619682#diff-5addde4ea142b5961e16cd909924dc4713c7077eb6ad01ac34a2c57b3e43cd4cR1172 maybe we could use parallel stream?

  1. while it indeed results in faster startup (drop from 75s to 25s for me) I wonder whether it wouldn't impose performance penalty during execution? Doesn't prepareCall() forces creation and caching of execution plan on db-level? If you don't see any downsides then I think we could flip that parameter to true by default.
Andrzej Wójcik (Tigase) commented 3 years ago
  1. How would a parallel stream help us? We need to call terminate on each Thread and then wait for them to complete. We could use a stream to terminate threads and catch them in the list..

  2. prepareCall() (same as prepareStatement() do not prepare execution plan on the server-side, nor preprocess query on the server-side, unless proper parameter is added to JDBC URI (ie. cacheCallableStmts, or cachePrepStmts). The only different that I know of is that prepareCall() queries for parameter types of a stored procedure and result knowns what should be set, while prepareStatement() just sends set parameter with passed types and "expects" them to match on the server-side. Will that introduce a penalty? I hope not, but I cannot tell from the quick test which I did.

As for flipping the parameter to true, we could and can do that.

Andrzej Wójcik (Tigase) commented 3 years ago

As for the switch, I've not tested all queries so I cannot tell if the workaround would always work, but Tigase started with TTS-NG configuration, so we could push it with this property set to true in config.tdsl and check if it works as expected and then decide to set to true by default.

wojciech.kapcia@tigase.net commented 3 years ago
  1. currently we go over them in sequence and if some threads take a little bit more then we wait for them instead of doing the check in parallel. though, we have shutdown of around 1,5s so I don't think this matters at all. (Slashing startup time would be more interesting and #issue #1210 is slated for 9.0)

  2. let's make it enabled in config.tdsl. It seems to work fine.

Andrzej Wójcik (Tigase) commented 3 years ago
  1. We are interrupting all threads (one by one) and then we check if treads are ready - if so we remove them from the set and wait 10ms then we recheck in a loop until the set is not empty. I would say this is ok, as we are "looping" one the single queue but waiting only if at least one thread has not finished (but they are being finished in parallel).

  2. That is already possible, I'm going to merge those changes to master branch.

Andrzej Wójcik (Tigase) commented 3 years ago

We do have to wait in MonitorRuntime but we are waiting for 2 threads to complete, so we can ignore that.

Andrzej Wójcik (Tigase) commented 3 years ago

Changes are merged to the master branch and it is possible to set 'useCallableMysqlWorkaround' = true in the config.tdsl file.

wojciech.kapcia@tigase.net commented 3 years ago

Thank you @andrzej.wojcik

Could you add information about this option to documentation and set TTS-NG to use it?

Andrzej Wójcik (Tigase) commented 3 years ago

Added documentation and enabled testing in TTS-NG

wojciech.kapcia@tigase.net commented 3 years ago

Thank you

issue 1 of 1
Type
Task
Priority
Normal
Assignee
Version
tigase-server-8.2.0
Spent time
8h 30m
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#1254
Please wait...
Page is in error, reload to recover