Slow database update

Hello, I'm using this code to set the kills of a player...I'm wondering why is so slow? Using it 5 times in a row freezes the server for 0.5 seconds, is there something wrong?

Code:java

protected void setKills(Player player, int kills, String minigameName) {
 
        String name = player.getName().toLowerCase();
        Statement statement = null;
 
        try {
            statement = connection.createStatement();
            statement.setQueryTimeout(MAX_TIMEOUT_SECONDS);
            ResultSet result = statement.executeQuery("SELECT * FROM " + TABLE_PREFIX + minigameName + " WHERE " + FIELD_NAME + " = '" + name + "' ;");
            result.last();
 
            if (result.getRow() == 0) {
                //create the profile if not existing
                result.close();
                statement.executeUpdate("INSERT INTO " + TABLE_PREFIX + minigameName + " (" + FIELD_NAME + ", " + FIELD_POINTS + ") VALUES ('"+ name + "', " + points + ") ;");
            } else {
                result.close();
                statement.executeUpdate("UPDATE " + TABLE_PREFIX + minigameName + " SET " + FIELD_POINTS + " = " + points + " WHERE " + FIELD_NAME + " = '" + name + "' ;");
            }
 
            statement.close();
 
        } catch (Exception ex) {
            ex.printStackTrace();
        } finally {
            if (statement != null) {
                try {
                    statement.close();
                } catch (SQLException ex) {
                }
            }
        }
    }

A query would be " UPDATE kills_pvp SET points = 30 WHERE name = 'filoghost'; ", this is the cause, is it wrong?

 

It is slow because the server has to wait for response from the database when you execute queries. You should run all that code in an asynchronous context, that way the main thread won't block waiting for response.

 

What Rocoty said, plus you should also be using prepared SQL statements, not creating new statements every time.

 

desht I'm not familiar with MySQL, however I only create one statement object
Rocoty I have to use this in an API who has the methods getKills() and setKills(). How would I manage getKills() asyncronously from an API?

Found a partial solution: getKills() is very fast, about 0.2 millis per request. setKills takes from 50 to 100 millis, that is 1-2 ticks. How about executing only setKills() asyncronously? Is that a clean solution?

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

You're creating one statement object every time setKills() is called, and you use it to run multiple SQL statements, each of which have to be compiled, every time. With a prepared statement, you create it once, and it lives for the lifetime of your plugin. You can then call it with parameters to substitute into the placeholders. I would suggest you spend a little time learning SQL if you're going to be writing plugins which depend on it.

Two possible ways:

1. Maintain a local lookup of kills by player (e.g. a Map<String,Integer>) as well as the persistent storage to your database. getKills() would read the internal map for fast lookup, and setKills() would add an entry to the map as well as scheduling an async update to your database. The final part of the puzzle is to have your plugin initialise the internal map from the database when it starts up - a one-time operation. For a player->kills mapping I would guess this table shouldn't be very large (unless you have tens of thousands of players), so that's a viable option.
2. Run getKills() asynchronously but provide a callback to the method which gets called when the data is actually returned from the database. You'd probably need to restructure your code somewhat if you do this, though, since getKills() won't return a result directly.

Meant to add before - if you're talking to an external database, it's not a safe assumption that your getKills() method will always be very fast. You're dependent on the load on the database server as well as the network load between your DB server and the CraftBukkit server, and any delays in getting data from the DB will cause lag if you're querying the database from the main thread.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Yeah it is.

 

Why not both? You can never be 100% sure how long it will take to get the response. It would also make your code somewhat inconsistent with itself. I don't see that as a clean solution at all, sorry.

 

desht Thanks for your detailed answer. I use a local database, so it should always be fast when using getKills(). If I execute only setKills() asynchronously in another thread, are there any problems with the MySQL database (Accessing to it from 2 different threads)?

 

MySQL should be able to handle access from multiple threads fine, but you have to keep in mind that no execution order is garanteed:

setKills(player, 500);
int kills = getKills(player); // will very likly not return 500, but the previously value

 

It is in such situations I would declare both methods as synchronized.

 

Rocoty it wouldn't work, because setKills() works in a different thread, right?

 

Then you loose the benefit of doing it in seperate threads in the first place.. If execution order is important I would let both methods be executed from the same, but seperated/async thread.

 

I don't think you understand. If the main thread never calls those methods at all, it would never block either. The synchronization is to retain execution order between the async threads that would call these methods.

EDIT: It would basically be the same as running just one async thread.

 

Okay, well, I had filoghost's decision in mind calling one method sync from the mainthread and the other one async. So I basicly agree with you that I also would prefer calling both async, either from one thread, one after the other, or from different async threads but synchronized.

 

I'm glad. Any of the methods would work more or less good, and it is nice to see other versions of the same opinion on things.

 

I found a good solution: both two methods are async, and the result is processed in an interface, like this:

Code:

setKills(Player player, int kills, new ResponseHandler() {
  public void onSuccess() {
  }
 
  public void onFailure(Exception ex) {
  }
 
})

This makes sense if you want to make a shop with kills: if setKills() is async and fails, you wouldn't know about that. The code is a bit longer, but it's executed async.