Async chunk modifications

I know, I know, async world modifications are bad, bad, bad. Let me explain the situation first.

The plugin I'm maintaining provides land protection in the form of chunk purchases. When you purchase a chunk, it sets redstone torches at each corner to mark the area you bought. When you sell the chunk, the torches go away (if they still exist). The problem arises when chunks are disbanded for player inactivity. Rather than manually go to each plot we have a command that deletes all chunks that a player owns. It works fine, but the torches persist. Not a huge issue, but they can quickly become an eyesore.

The solution is to run my setCornerTorches(false) method on each chunk that is being disbanded. The problem of course is that those chunks aren't guaranteed to be loaded. Loading them, removing the torches, then unloading and saving them is trivial, but if you are doing that with 600 chunks its going to be a huge lag spike (or crash) because this is all happening in the main thread.

My idea is to first test if the chunk is loaded. If so, no problem, just run the torch removal method on it synchronously. Its very efficient so it shouldn't cause lag because the chunk is already loaded. If the chunk isn't loaded, add it to a queue of chunks for torch removal. Pass that queue to a thread that loads the chunk, removes the torches, then unloads the chunk and saves it. Since the chunk was unloaded, there are guaranteed to be no players in that chunk, and there's no chance of it being concurrently modified.

Will this work, or are there other issues I'm not thinking about?

 

It might be easier to spread the work out- make a repeating task (at 10 ticks or whatever) with a list of chunks to delete, and have it load/remove torches on one chunk per iteration.

As long as the chunk GC stays ahead of you, it should be ok, I think, and probably safer.

 

Make all stuff you can async and queue other jobs that should be sync and commence them at slow rate.

 

That's a great idea! And then I could just kill the repeating task once the list was empty. I may do just that.

But I am curious about the safety issues, because I'm still a bit new to running things asynchronously. What potential issues could arise from doing it in the way I described?

 

Well it is a little pain. For example when you reload your server you should stop those tasks (kill them by ID so store those IDs) and then start again. I think that will be important in your case.

 

Chlorek is probably right, since this will take a potentially very long time to run.

But it also sounds like you're separately storing the data (like regions without owners) that you'd need to re-run the job, so maybe it's ok if it just stops before completion?

Doing async chunk modifications is probably just a bad idea no matter how careful you are about it. This could range from Bukkit writing to the region files at the same time from two different threads, corrupting them- to just deadlocking the whole server if you hit some kind of synchronization block.

 

Thanks guys! I think I'll test out running the repeating task with delay to see how that works, as speed of completion really isn't critical for this task. It'll be later this weekend before I get to try this (hopefully) but I'll let you know how it goes.

 

CraigParton
My idea to achieve this without risk of any undefined ending:
When you remove region, mark chunks as those to be checked. Then you can catch chunks loading and take care of removing what you want.

 

Ah i see, store a list of chunks that need to be checked for torch removal, then hook the chunk load event and remove the torches the next time the chunk is loaded naturally (if its on the list). That wouldn't place any additional strain on the server since the chunk is being loaded anyway. The only tradeoff is a little overhead for loading new chunks, but I could thread the database check and use a sync callback to remove the torches if that chunk is on the list.

That is a great idea too, not sure how hard that would be to implement but it doesn't seem that difficult. Thanks!

 

Do not do it async.

If you have more than one, just do one per server tick. Schedule a repeating task that slowly removes them all. Instead of slowing one tick down by a ton, slow 100 ticks down by a little bit so it's less noticeable (and less "laggy").