Folia icon indicating copy to clipboard operation
Folia copied to clipboard

World load/unload api

Open masmc05 opened this issue 1 year ago • 11 comments

Reimplemented the world load/unload api

Was working fine a few days in production, but still will keep this a few more days in draft in case of issues

Had to add a little new concept that wasn't specified anywhere, after we stop the activity and any ticking in the world, so there aren't any regions in the world that can tick the world, or have tick threads, we transfer the rights over the world to global tick thread, and we should try to only read the data that was frozen after halting the activity

The unloading happens by freezing the state of the world, stopping any regions from ticking, but not removing them, that allows us to serialize the state of the world without worrying that something could alter any data

Loading of the world is lightweight, and is global tick only just to synchronize with unloading, it doesn't really needs that, just to be sure Unloading has to be done on global thread, plugins should try to disable keep spawn chunks and wait a bit after players left the server so folia could save and remove all regions from world, and so without the necessity so save any data on global thread, but that is the case only if hight amounts of load/unload will happen

Even if unloading a world that don't have any regions anymore can happen almost instantly, increasing performance, unloading a world that still have regions left is also supported

masmc05 avatar Apr 26 '23 16:04 masmc05

Will this be implemented eventually?

Aitooor avatar May 07 '23 20:05 Aitooor

I think it's good, didn't have any issue with that

masmc05 avatar May 13 '23 19:05 masmc05

There are many significant issues with this that are not addressed:

  • teleporting into worlds that may or may not be unloading (this includes player login) is just not handled, which is unacceptable
  • interactions with the entity scheduler or region scheduler, this includes internal access as well as API access
  • waiting until all regions are halted (in your code this is done incorrectly due to threading issues) is not good enough, as new chunk holders may be created asynchronously by ticket additions which may create other regions
  • halting regions by killing them is error prone, as the regioniser expects (and will throw) if attempting to merge / split dead regions, this includes general maintenance through addChunk/removeChunk
  • using the global tick thread to save the chunks is inappropriate as the global tick thread is not supposed to be doing expensive work, as it is maintaining the time for the worlds as well as being a fallback for processing tasks if there are no other tickable regions active. I do agree that the global tick thread is responsible for scheduling world loading / unloading though
  • realistically, there should not be any hacks to support reading other region's data during unloading as this imposes maintenance burden. the shutdown thread is an example of how to avoid this

There may be more, as I've not actually tried doing this myself. It's insanely hard to do right, which is why it's currently unsupported entirely.

Spottedleaf avatar May 13 '23 23:05 Spottedleaf

this includes player login

Player login has to go trough global thread, which is doing the unload, so I don't see possible issues with that since a player login and a world unload can't happen in parallel

using the global tick thread to save the chunks is inappropriate as the global tick thread is not supposed to be doing expensive work, as it is maintaining the time for the worlds as well as being a fallback for processing tasks if there are no other tickable regions active. I do agree that the global tick thread is responsible for scheduling world loading / unloading though

The part of scheduling world loading / unloading is an interesting part, I think I'll rework this a bit to be a scheduling instead of immediate unloading, this should also resolve other issues since we can make regions save and unload themselves, need to think a bit about new chunk loading which will be even harder

global tick thread is not supposed to be doing expensive work

As I said, if there is no player and no keep spawn chunk loaded, there is no region, and so no work to do, the unload is instant, which plugins should aim for

In general yeah, missed those, will need to dig even deeper into this to see all possible changes while the unload process is happening

masmc05 avatar May 14 '23 08:05 masmc05

While testing this, if the world for any reason was to be unloaded then the players inside the world, they would get "soft locked". the server completely breaks as the players is attempted to be removed on join but fails and in doing so the server freaks out and breaks the commands and much more. "java.lang.IllegalStateException: Player is already removed from player chunk loader".

Best way i found a fix around it was leave the world offline and when the player rejoins after a restart then it will be fixed.

wildmaster84 avatar May 21 '23 19:05 wildmaster84

While testing this, if the world for any reason was to be unloaded then the players inside the world, they would get "soft locked". the server completely breaks as the players is attempted to be removed on join but fails and in doing so the server freaks out and breaks the commands and much more. "java.lang.IllegalStateException: Player is already removed from player chunk loader".

Best way i found a fix around it was leave the world offline and when the player rejoins after a restart then it will be fixed.

hmm, how the players went there? there is a check that should block unloading of a world with players

masmc05 avatar May 22 '23 10:05 masmc05

if they went there while unloading then yes, that's something i work on right now (but didn't happen on testings)

masmc05 avatar May 22 '23 10:05 masmc05

image this is how im unloading worlds, currently if the owner of the world leaves then the players are removed from the world, but if a random player leaves then they will glitch into a broken state. i have also included the method on how i am sending players to the worlds. image

wildmaster84 avatar May 22 '23 10:05 wildmaster84

Maybe only load/generate worlds before players can join? Seems like the safest for now

SlayorPlayz avatar May 22 '23 22:05 SlayorPlayz

Error when using .teleportAsync to teleport someone to another world: https://paste.gg/p/anonymous/f56e4421299a4456aee05bad603afc97

SlayorPlayz avatar May 27 '23 15:05 SlayorPlayz

Without the issues I laid out being resolved I'm going to close this PR.

Spottedleaf avatar Jul 01 '23 20:07 Spottedleaf