Petition to remove the /reload command

Discussion in 'Bukkit Discussion' started by Afforess, Oct 27, 2011.

Thread Status:
Not open for further replies.
  1. Offline

    Afforess

    The reload command in bukkit is diabolical, and should be removed. I've stated this many times in private before, but I'm often surprised by the number of people who are unaware of the problems with reloading.

    Memory Leaks:
    To understand why the reload command is terrible, you have to understand how it works. /reload disables all active plugins, and then, creates a new instance of every plugin in the plugins folder. It does not reactivate the old, disabled plugins. It creates a new copy. Due to the way Java works, and how Bukkit implements it's classloaders, this causes a memory leak. This is easy to confirm with most java profilers. The memory leak is not super significant (I'd guess at most, 5mb per /reload), but it does exist. Unfortunately, Java's eccentricities make it pretty much impossible to fix this memory leak.

    The effects of the leak can be viewed by my profiling some months back. Nothing has changed with plugin loading since these screenshots were taken.

    Server running BukkitContrib without any reloads.

    Server running BukkitContrib 4 reloads later. (Plugins loaded 5 times total, 4 reloads + 1 startup)

    All of the classes and any static variables remain in memory.

    Poor Plugin Design:
    Memory leaks are not the worst part about the reload command. Many, MANY plugins handle reloads poorly, or not at all. I'm certain most server admins are painfully aware that when they use /reload, the console will spit out at least one error, usually more. Exceptions are extremely common with /reload. And before you begin to berate plugin developers for simple mistakes, know that it is often extremely difficult to handle reload situations perfectly, especially the more complex the plugin. The most complex case I am aware of is Spout, where we have literally, thousands of lines of code simply to handle the /reload command. And Spout still doesn't handle reloads perfectly. Plugins that use multiple threads to achieve their tasks (usually plugins using some form of SQL, or persistent storage....) will also fail to end their threads after the plugin shuts down. This means your server could be wasting CPU cycles for extra threads that are not needed or used.

    Lag, Lag, Lag:
    Assuming that your server has only selected the highest tier plugins, of exceeding quality that handle reload flawlessly, and has gigabytes of extra memory to handle any leaks from bukkit, reload will still lag for several seconds after you issue the command. It is not uncommon for players to be kicked for fly hacking, moving to quickly, or just to read time out from lag. You might as well have used the 15 seconds it takes to RESTART the server instead of reloading it.

    There are several plugins made specifically to offer a way to safely RESTART your server instead of reloading it, and I encourage admins to use them:

    http://forums.bukkit.org/threads/ad...w-v0-3_2-full-server-restarts-818-1060.20039/
    http://forums.bukkit.org/threads/ad...-automatic-rebooting-and-commands-1000.24652/
    http://forums.bukkit.org/threads/ad...t-down-at-the-same-time-every-day-1248.26101/
    http://forums.bukkit.org/threads/ad...detection-auto-saves-remote-console-1317.674/

    I'm personally inclined to simply block the reload command from any of my plugins, but I expect that would make admins unhappy. So instead, I'm trying to raise awareness of why the command is bad, what can happen when you use it, and alternatives for it.
     
  2. Offline

    mindless728

    well realistically you could do that, yeah there would be some backlash, but as long as you explain the situation it shouldn't be much of an issue. Not to mention this could be the price of having a good plugin that stops you from doing (potentially) stupid things.

    honestly i wasn't aware of this, but at the same time i never used reload i always did a clean restart.

    EDIT: /signed
     
  3. Offline

    Vhab

    /signed
     
  4. Offline

    Kainzo

    I can see the need to remove it based on the current implementation - the majority of these issues stem from plugin developers not properly unloading the plugin.

    P3/wg/we/cmdbook are several that I see not handling their async tasks correctly.

    Now, from a large server administrator - I don't see much 'weight' when we kick reloads - other than it taking forever. It's not good practice and I doubt it will ever be to do it. It's useful on test servers when I dont want to stop/restart
     
  5. Offline

    Greylocke

    /signed
    I was floored when, after testing some plugins and doing numerous /reloads, I saw how completely trashed my memory allocation was. I've never used /reload since.

    Plugin developers should be encouraged to always include a method to reload their own config files. This will help to remove the need for a server /reload command.
     
  6. Offline

    r3Fuze

    When i bugtest plugins I reload alot and could easily get to 100 /reloads, now that I've read this I'll get some plugin that handles it better.

    /signed
     
  7. Offline

    petteyg359

    Remeber to null stuff in onDisable()... I think I've prevented my own plugins from leaking any more than the bare Class takes when Bukkit doesn't let it get garbage-collected :)
     
  8. Offline

    Mukrakiish

    There's very little reason at least from my own Admin standpoint to ever totally reload the server. Other then being lazy and quick-implementing a plugin that was dropped in without having to stop/start (which again is lazy and just not a wise thing to do), each plugin should have its own /reload command in itself to load the related config. Not a server-wide reload for all configs. If your that far along...do stop/start.

    /signed
     
    Walker Crouse likes this.
  9. Offline

    TopGear93

    /signed
     
  10. Offline

    Cedar

    /signed
     
  11. Offline

    ZNickq

    /signed
     
  12. Offline

    saul100

    /signed
     
  13. Offline

    sukosevato

  14. Offline

    Coelho

    Plugin developers need to come to their senses and start unloading the plugin correctly. More than half of the plugins currently available don't do this, and that is the main reason why the reload command is terrible. Even though the reload command itself has issues, it is still useful for lazy server administrators like me. I have no problem with it.
     
  15. Offline

    jblaske

    /signed

    I would rather see it changed from a global disable and re-newing of plugins, to just triggering a reload event taht plugins can hook into, and execute their own reload code.

    This way if you need to reload all configs the /reload would do that. But if you only wanted to reload one plugin, you use that plugins reload command.
     
    DeanDip likes this.
  16. Offline

    mindless728

    so you have no problem with a command that even under the best of circumstances causes a small memory leak that can add up over time.......maybe you shouldn't be running a server
     
  17. Offline

    Orcem12

    Get it out...
    /signed
     
  18. Offline

    Coelho

    I'm not going to reload the server every minute: probably once or twice a day. It doesn't add up to much at all as I see no performance loss or high resource usage.
     
  19. Offline

    yttriuszzerbus

    If you don't like reload (I don't much either) then why not just not use it?
     
    xYourFreindx and halley like this.
  20. Offline

    Don Redhorse

    hmm so how do you code your plugin correclty for reload? afaik there is no way to unregister a listener for example..

    keeping that aside.. why not implement another method for the JavaPlugin onReload and let that trigger with /reload

    let the plugin developers handle the reload command correctly for their plugin..
     
  21. Offline

    Afforess

    @Don Redhorse

    I've been thinking in that respect. I think the best solution would be a 2 part solution.

    Have Bukkit offer an official /restart command that stops and starts the server again. Then change how the reload command works drastically.

    Instead of reloading all plugins, have plugins be able to say whether they can be reloaded in the plugin.yml. If they can, disable the plugin safely, remove it's listeners and cleanup, then enable the plugin again (instead of creating a new plugin).
     
  22. Offline

    obnoxint

    I think, the best solution is to use class variables only and to "null"ify them all, when onDisable() is called. This minimizes the potential of memory leaks caused by instances of the JavaPlugin class. All referenced class instances will be caught by the GC eventually.
     
  23. Offline

    rmb938

    /signed
     
  24. Offline

    Darq

    @yttriuszzerbus
    Even though some Admins might not use it, a LOT of others will, and Devs compensate for that by writing code that makes /reload work a bit better. With /reload completely out of the picture, Devs won't have to work their ass off writing code to make a command that they really shouldn't have to worry about work! As Afforess said, they have literally thousands of lines of code just to handle /reload better. And as he said, they could just not care, and remove /reload support from all their plugins, but they'd receive quite a bit of backlash from doing that, when Admins can't reload their server without a bunch of errors.

    /signed
     
  25. Offline

    Kaikz

    At least allow us to hotswap plugins like in hMod.
     
    Darq likes this.
  26. Offline

    Afforess

    That's actually how reload works right now, and why it doesn't work well.
     
  27. Offline

    Darq

    *wishes he didn't like Kaikz' post =3*
     
    Kaikz likes this.
  28. Offline

    Kaikz

    sadface.
     
  29. Offline

    BioRage

    /signed.
     
  30. Offline

    Don Redhorse

    that can be easily done by a script, which than allows to really clean out memory... I doubt you can do that with an internal java process or?
    hmm... why make it that complicated and just not use onReload similar to onEnable?

    developer can than code the correct handling of the plugin into it, plus a reload will not do an onEnable / onDisable anymore AND an option to unregister listeners..

    which reminds me to make sure that my listeners are singletons (if that even works)
     
    DeanDip likes this.
Thread Status:
Not open for further replies.

Share This Page