CloudNetService / CloudNet

A modern application that can dynamically and easily deliver Minecraft oriented software
https://cloudnetservice.eu
Apache License 2.0
367 stars 115 forks source link

chore: Increase timeout for process termination #1422

Open GiantTreeLP opened 1 week ago

GiantTreeLP commented 1 week ago

Motivation

Current Java Edition Minecraft servers, especially those that are forks of the vanilla Minecraft server software, take some time to properly stop and save all resources, i.e. worlds, inventories and other state to disk. Prematurely terminating those processes leads to worlds not being saved at all, world corruption due to the interrupted save process and unloadable worlds. In addition this leads to lock files being left that don't belong to any process anymore.

Modification

Increase the timeout to 60 seconds. This gives servers more time to properly store all files and shutdown whilst still cleaning up servers that may have become stuck whilst shutting down.

Result

Server processes are now given more time to properly shutdown.

Other context

See the discussion in Discord starting from here.

DasBabyPixel commented 1 week ago

I feel like this should be configurable instead of fixed 60s (or 5s) Best would be a setting for the task imo. Some lobby template servers only need 5s and should be killed after, some heavily modded static server might need more. Yet you also don't want to wait for a whole minute for broken lobby server to be killed.

derklaro commented 1 week ago

Yes, the configuration should be available per-service, but that's not a simple task to do as the task configuration is not available to the service, unless we provide the timeout as a variable in the service configuration or retrieve the task from the task name provided to the service configuration.

DasBabyPixel commented 1 week ago

Yes, the configuration should be available per-service, but that's not a simple task to do as the task configuration is not available to the service, unless we provide the timeout as a variable in the service configuration or retrieve the task from the task name provided to the service configuration.

what's the problem with providing it as a variable in the service configuration?

derklaro commented 1 week ago

The thing is: sure, we could just throw the variable in there and move on, but there are more places that have variables that cannot be set per service, and need to be accessed via the task (which shouldn't be the case). One example is the required permission for a task or the smart configuration. So imo it's better to delay this a bit and find a better solution on how we can handle the variables and then merge this :)