Use separate thread SQL

Nogtail Something i found on the interwebs http://cleancodedevelopment-qualityseal.blogspot.de/2012/10/understanding-callbacks-with-java.html

You could of course just "use them" as mcbans did it but i find this a bit "unclear" as you can't just "see" how it works.

Typically you should build a handler that can do stuff in its workspace. I call them modules. A class should never do too much or it gets too complicated very fast and you mixup things that could better be elsewhere since its then easier to extend. If you mixup too much you need to change depending code too. That you want to avoid since this can affect critical parts you cant just "change".

If something needs to be done create a class .. if something can not be done by the same class cause it would need more data and its not related create something that "can provide" it.

In this case we would need a DataProvider that can fetch data and execute events based on the result so you can work with them without freezing other threads too much while getting the data.

The target is that you simply listen to your custom events lets say "PlayerDataChanged" and implement the logic there instead doing this inside the DataProvider or "inbetween".

Hope that helps.

::Edit::

On a second thought whilst i really would like to see this strategy in action you could implement an easier way as this is pretty much not so simple to do as you might have checked.

Simply utilize the event system. Create custom events and utilize them to get what you need.

For example: You want to get PlayerInventory from database when a player changes his inventory contents then store it to your database again.

I like to name them with my plugin name prefixed and either use Bukkit Event names or custom ones that describe what i want to do with it. This way we can identify them better.

BukkitEvents involved:
InventoryOpenEvent

Custom Events involved:
YourPluginNameInventoryPreOpenEvent
YourPluginNameInventoryRequestEvent
YourPluginNameInventoryRequestedEvent

So you listen on InventoryOpenEvent (bukkit event) then call your Custom Event since you want to redirect it to your own events to structure calls.

in YourPluginNameInventoryPreOpenEvent you can do stuff that should be done before we continue.

Then call InventoryRequestEvent while passing the name of the player to callEvent. How that works can be read here: http://wiki.bukkit.org/Event_API_Reference#Creating_Custom_Events

The idea is to only transport what we need.

Now in YourPluginNameInventoryRequestEvent do call an async task that fetches Data. Inside this task run a task that is synchronous. Let it call YourPluginNameInventoryRequestedEvent and pass the data from the DB.

This event can then do whatever it should do. Could just display how many dirt blocks are in the inventory since we now have all the information from the database.

Why calling events instead of methods? If you do call an event you give the system a chance to react and also other plugins could also listen on that event plus you do not have to use a monitor thread that polls for changes.

 

I could give some short code snippets from my rewrite of Paintball: untested, not yet debugged and still in progress, but maybe useful as example:

I have some "DataManager" class:

PHP:

private final DataProviderI dataProvider;
private final Thread worker;
private final BlockingQueue<DataTask> tasks = new PriorityBlockingQueue<DataTask>();
private final Object TASK_LOCK = new Object(); // gets locked while a task is being executed, useful for methods which need to wait for the current running task to complete
 
public DataManager() {
    // initialize data provider:
    String databaseMode = PaintballAPI.getInstance().getMainConfiguration().get(ConfigSettings.DatabaseMode);
    if (databaseMode.equalsIgnoreCase("mysql")) {
        //TODO not yet implemented..
      //this.dataProvider = new MySQLProvider();
        this.dataProvider = new SQLiteProvider();
    } else {
        // default: sqlite
        this.dataProvider = new SQLiteProvider();
    }
   
    // start worker task:
    this.worker = new Thread(new Runnable() {
       
        @Override
        public void run() {
            while (true) {
                try {
                    Runnable nextJob = tasks.take();
                    synchronized (TASK_LOCK) {
                        nextJob.run();
                    }
                } catch (InterruptedException e) {
                    // the thread was interrupted while we are waiting for more work, which doesn't seem to be there currently -> let's finish then
                    break;
                }
            }
            Log.info("Database thread was successfully stopped.");
        }
    });
    this.worker.start();
}
 
public void addTask(final DataTask task) {
    Validate.notNull(task);
    if (!tasks.offer(task)) {
        throw new IllegalStateException("Database-Job-Queue seems to be full! This shouldn't happen.. :/");
    }
}


I use a "PriorityBlockingQueue" here to add and retrieve "DataTask"s (Database requests) in a thread safe manner and at the same time give the different possible tasks a order depending on their "priority". I have not yet tested this and have no idea how much overhead this will create for me. Also, I am currently limited here with 1 "worker" thread: which is no problem for me right now as sqlite is limited to 1 write-access at a time anyways, and I wanted to keep it simple for now, but it may be improveable in the future, especially for read-data-tasks, which are not dependent on the current write-tasks.

My base DataTask class looks like this:

PHP:

public abstract class DataTask implements Runnable, Comparable<DataTask> {
 
    public enum Priority {
        HIGH(1),
        NORMAL(0),
        LOW(-1);
 
        private final int priority;
 
        private Priority(int priority) {
            this.priority = priority;
        }
 
        public int getPriority() {
            return this.priority;
        }
    }
 
    /*
    * Known DataTask priorities:
    * LoadAllPlayerStatsTask - HIGH
    *
    * SaveAllPlayerStatsChangesTask - NORMAL
    * SavePlayerStatsChangesTask - NORMAL
    *
    * RefreshPlayerStatsTask - LOW
    *
    *
    */
 
    private final Priority priority;
 
    public DataTask(Priority priority) {
        this.priority = priority;
    }
 
    public Priority getPriority() {
        return this.priority;
    }
 
    @Override
    public int compareTo(DataTask otherTask) {
        return otherTask.getPriority().getPriority() - this.priority.getPriority();
    }
 
    @Override
    public abstract void run();
 
}


And a potential example for such a "DataTask" which also takes a Callback could be this:

PHP:

public class LoadUUIDSForPlayerNameTask extends DataTask {
 
    private final String playerName;
    private final Callback<List<UUID>> callback;
 
    public LoadUUIDSForPlayerNameTask(String playerName, Callback<List<UUID>> callback) {
        super(Priority.HIGH);
        Validate.notEmpty(playerName);
        this.playerName = playerName;
        this.callback = callback;
    }
 
    @Override
    public void run() {
        final List<UUID> uuids = PaintballAPI.getInstance().getDataManager().getDataProvider().getPlayerUUID(this.playerName);
 
        // continue sync and run callback when done:
        Bukkit.getScheduler().runTask(Paintball.instance, new Runnable() {
 
            @Override
            public void run() {
                if (callback != null) {
                    callback.onComplete(uuids);
                }
            }
        });
    }
}


The dataprovider will simple execute a sql statement which does a lookup in the player data table to find all existing player-UUIDs for a given playerName. I use this for example to print statistics data for a given playerName on requests, for example as reaction to a command. My database will store it's data for unique player-uuids (there can exist multiple player-data-rows for players with the same playerName, so this returns a list of the potential candidates for the requested data).

Now in my "PlayerManager" class I will provide a method to request those uuids for a given playerName:

PHP:

    public void getUUIDListFromNameAndRun(final String playerName, final Callback<List<UUID>> runWhenDone) {
        Validate.notNull(playerName);
        Validate.notNull(runWhenDone);
        // check if there are already requests for this data existing:
        UUIDRequests requests = uuidsForNameRequests.get(playerName);
        if (requests != null) {
            // add request:
            requests.addListRequest(runWhenDone);
        } else {
            // let other requests know that the data is already in the progress in being gathered:
            requests = new UUIDRequests();
            uuidsForNameRequests.put(playerName, requests);
            requests.addListRequest(runWhenDone);
 
            this.startUUIDListRequest(playerName);
        }
    }
 
    private void startUUIDListRequest(final String playerName) {
        assert playerName != null;
        // init async task to get the known uuid(s) for the given player name from database:
        PaintballAPI.getInstance().getDataManager().addTask(new LoadUUIDSForPlayerNameTask(playerName, new Callback<List<UUID>>() {
 
            @Override
            public void onComplete(List<UUID> uuids) {
                // provide the data to all other requests which happened in the meantime:
                uuidsForNameRequests.remove(playerName).handleData(uuids);
                // no caching here: uuid<->playernames mappings will be cached once we are getting and caching PlayerStats as well
            }
        }));
    }


I am also caching some of the playerName <-> uuid mappings, for example those of the current playing players or those for the cached playerstats.
And I am "catching" the incoming uuids-for-name-requests in a map while a such a request is already being in progress / the data already being gathered from the database. All requests / their callbacks which happen in this (short) time while the Thread from the DataManager is executing the task(s) will be stored and when the data becomes available for the first request (which initiated the new DataTask) the data will be shared with all the other waiting requests as well.
I am not completly sure though, if this requests catching is actually worth it, as most such requests will be done pretty fast anyways I guess, and generally there won't happen many of those requests.. But well, I am not even yet in "alpha" with this and it's far from being runable, so I have enough other things to currently care about.

Hope this is useful as example. Bare in mind that I am doing all this "structuring" for the first time as well and am pretty new in this topic..

Between, while I use "Callback"s here I have another DataTask (the one which initially makes sure that the entries for a player are being exisiting in the database, and created if they are not), which calls an custom event instead when being done.
That's because I generally only have 1 such event for a player, shortly after him joining the server, so listeners which listen for this custom "PlayerDataAddedEvent" don't have to verfiy if they currently are in need for this information, because they always are (otherwise they wouldn't listen for this event; there is no other possible situation where and when this information, that the player was added to the database and is now 'ready', could be generated or needed). One such listener for example handles the "auto-join" logic, after the player data was added/verified in the database.

 

lycano For the saving you mean? Because I can!

 

blablubbabc do you think a minecraft plugin is a seperate program? I do really not understand why people try to use threads on their own.... even though you might think you need it here you could also just let it run as an async task that repeats itself.

What do you do when the plugin will get stopped or events are triggering and another event was fired before? Do you want to implement many events just to make your thread "safe" to use since you would have to cancel a task at some point IF something happens you do not want to let it happen.

Bukkit is event based in terms of plugins ... trying to put stuff in a thread will not fix a problem.

 

Yes, I could do that. However, I will have the same "problems" to shutdown this async bukkit task, like I have with my own, dedicated thread: to shut it properly down you have to wait for the remaining task(s) to finish, before I continue disabling the plugin and shutting down database connections etc.. "asyncBukkitTask.cancle()" will simple mark it for "finisih after you are done with your current run". You still have to wait for at least this last task to finish.

So this is what I will be doing:

Code:

    // called by the main thread when the plugin is disabled:
    public void shutdown() {
        this.worker.interrupt();
        // Let's give the worker time to finish it's work before continuing shutting down:
        final long start = System.currentTimeMillis();
        while (this.worker.isAlive()) {
            try {
                Thread.sleep(5);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            // wait max 5 seconds then shutdown anyways..:
            if (System.currentTimeMillis() - start > 5000) {
                Log.warning("Waited over 5 seconds for " + (this.tasks.size() + 1) + " remaining database tasks to complete. Shutting down anyways now..");
                break;
            }
        }
        long timeWaited = System.currentTimeMillis() - start;
        if (timeWaited > 1) Log.info("Waited " + timeWaited + "ms for the database tasks (probably stats saving) to finish.");
       
        // shutting providers down:
        this.dataProvider.onShutdown();
    }

So this will freeze the main thread (for max. 5-10 seconds, in general I guess it will only take a few milliseconds) on disable to give the async worker thread the chance to finish it's remaining work

I am not completly sure what you meant with those "events", but I am definitly sure that you have the same problems with bukkit's async tasks, as those tasks are also simple passed to one of the available underlying threads for execution.

 

blablubbabc your thread runs in your plugin scope. The Bukkit Thread will run inside the scheduler scope. Why i say "you should not do this" is kinda simple. I believe that the bukkit scheduler knows best when it will run something where you simply say "okay i have this thread lets run it".

By using a queue that only takes care of your plugin decisions you dont give other tasks a chance which you really should avoid.

Also by shutting it down anyways after 5 seconds (if its not cleanly shut down) .. how can you tell that a database call does always take less than 5 seconds? Maybe i is good to wait for it to finish a little bit longer in certain cases.

You know there are so many conditions you shouldn't really care to fix em all since this will not be possible - let the bukkit scheduler do it.

 

lycano

The bukkit scheduler maintains one queue for all sync and async tasks, and it simple goes over this queue each tick and runs each task which needs to run by looking on the delay in ticks. If the task is sync it simple calls "run()" on that task. If the task is async it redirects it to the executor -> to one of the threads from the thread pool.

So the async repeating task is triggered at max. 1 time per tick (1 time each 50 ms; 50 ms in which I could probably run multiple database tasks..), and it waits for all the other sync task which are in front of it in the queue to finish running, wasting again a little time in which the async task could already run in a different thread, at the same time.

To the 5 seconds waiting: I will probably use a higher limit like 10 seconds, or I assign each task some sort of estimate max. time to calculate the max. remaining time depending on the actual remaining tasks.
This limit is only meant as absolut max. limit and as sort of protection against an infinite running worker thread.. I currently don't expect it to take longer than those 5 or even 10 seconds, but in most cases only to take a few milliseconds, most of the time not even 5 ms (at least this was the maximum I was observating so far).

But no matter how I will determine the time to wait: I would have the same problem with the async bukkit task.