PlayerActivityListener#onPlayerLeave - do not do (basically) synchronous network calls

[deanveloper]

https://github.com/track/plugin/blob/35cd4cc7cf43051ef2e18ab4ff55c8dd52c35544/src/main/java/net/analyse/plugin/listener/PlayerActivityListener.java#L80

Bukkit.getScheduler().runTaskAsynchronously does not actually run the task on a new thread, it just defers the work to the chat thread. This means that on the event of a mass user disconnect, the singular thread will hang because AnalyseSDK#sendPlayerSession is a synchronous networked call.

I am making this issue because someone had asked me and some others why their server was crashing under a stress test when users left their server, and upon further investigation it seems to be because of the Analyse plugin. I believe that this line is the culprit.

There are two possible solutions for this. You can either

1. Replace Bukkit.getScheduler().runTaskAsynchronously with a more proper ScheduledExecutorService. This is a good temporary and easy fix.
2. Have AnalyseSDK networked calls return CompletableFutures (which are similar to Javascript Promises). This is probably going to require a bit more work, but will result in better code down the line and reduce tech debt.

[deanveloper]

UPDATE - apparently bukkit actually does proper async schedulers for runTaskAsynchronously, which was not true back when I did minecraft dev LMAO.

Anyway, the issue with stress testing still exists - a server crashed when 80 users all left at the same time. There are likely multiple causes for this (likely a mix between slow API response time and many parallel requests), but the best way to combat the issue would likely to either spread out the requests over time (easy, but short term) or to do bulk requests (a bit more difficult, but a longer-term solution)

[heychazza]

Hey Dean, thanks for your time when creating this issue. I've since fixed the command issue that you mentioned within the other issue you referenced.

As for the issue with crashing, all network requests are sent async so this certainly shouldn't be an issue, however - I'm not sure what the benefit of CompletableFutures would have to prevent this "crash". This is the first I've heard of this across the hundreds currently using our service.

If there is a solution you know of, it would be awesome to direct a pull request for this to our SDK. I have no experience with this aspect of it, and as a 1-man band I would certainly appreciate the support.

In the meantime, I'll close this issue as it is the only time I've heard of it. I really appreciate your time nevertheless.

[Penple]