[Fix] Test & /raw/server/ improvements (#776) by Fuzzlemann

* Optimizing the loading of the network page and raw server data page

-> Loads the nicknames in bulk instead of one by one
-> Performance gain with the raw data 45719ms vs 10054ms
-> Scales up with more even more users

* Adding UsersTable#getUUIDsAndNamesByID so when you want to get the UUID and the player name you do not have to execute UsersTable#getPlayerNames and UsersTable#getUUIDsByID
-> Performance improvements

* Fixes the test failures

* Renamed method names so they correctly show the real DB used
True fix of the tests (no fail after ~3500 passes)
  -> Implementation of TestRunnableFactory
  -> Removal of the timeout on CommonDBTest#testSaveCommandUse which is way too sensitive to the testing environment (disk speed, etc.)
This commit is contained in:
Fuzzlemann 2018-11-03 21:09:12 +01:00 committed by Risto Lahtela
parent 471a830c9f
commit 81533718cd
13 changed files with 209 additions and 16 deletions

View File

@ -29,7 +29,7 @@ import com.djrapitops.plan.system.database.databases.operation.*;
*/
public abstract class Database {
protected boolean open = false;
protected volatile boolean open = false;
public abstract void init() throws DBInitException;

View File

@ -145,13 +145,13 @@ public class H2DB extends SQLDB {
@Override
public void close() {
super.close();
stopConnectionPingTask();
if (connection != null) {
logger.debug("H2DB " + dbName + ": Closed Connection");
MiscUtils.close(connection);
}
super.close();
}
@Override

View File

@ -134,10 +134,11 @@ public class MySQLDB extends SQLDB {
@Override
public void close() {
super.close();
if (dataSource instanceof HikariDataSource) {
((HikariDataSource) dataSource).close();
}
super.close();
}
@Override

View File

@ -288,6 +288,10 @@ public abstract class SQLDB extends Database {
public abstract void returnToPool(Connection connection);
public boolean execute(ExecStatement statement) {
if (!isOpen()) {
throw new DBOpException("SQL Statement tried to execute while connection closed");
}
Connection connection = null;
try {
connection = getConnection();
@ -328,6 +332,10 @@ public abstract class SQLDB extends Database {
}
public void executeBatch(ExecStatement statement) {
if (!isOpen()) {
throw new DBOpException("SQL Batch tried to execute while connection closed");
}
Connection connection = null;
try {
connection = getConnection();
@ -346,6 +354,10 @@ public abstract class SQLDB extends Database {
}
public <T> T query(QueryStatement<T> statement) {
if (!isOpen()) {
throw new DBOpException("SQL Query tried to execute while connection closed");
}
Connection connection = null;
try {
connection = getConnection();

View File

@ -137,12 +137,13 @@ public class SQLiteDB extends SQLDB {
@Override
public void close() {
super.close();
stopConnectionPingTask();
if (connection != null) {
logger.debug("SQLite " + dbName + ": Closed Connection");
MiscUtils.close(connection);
}
super.close();
}
@Override

View File

@ -24,6 +24,7 @@ import com.djrapitops.plan.data.store.mutators.PerServerMutator;
import com.djrapitops.plan.data.store.mutators.PlayersMutator;
import com.djrapitops.plan.data.store.mutators.SessionsMutator;
import com.djrapitops.plan.data.store.objects.DateObj;
import com.djrapitops.plan.data.store.objects.Nickname;
import com.djrapitops.plan.data.time.WorldTimes;
import com.djrapitops.plan.system.cache.SessionCache;
import com.djrapitops.plan.system.database.databases.operation.FetchOperations;
@ -123,6 +124,7 @@ public class SQLFetchOps extends SQLOps implements FetchOperations {
Map<UUID, Integer> timesKicked = usersTable.getAllTimesKicked();
Map<UUID, List<GeoInfo>> geoInfo = geoInfoTable.getAllGeoInfo();
Map<UUID, List<Ping>> allPings = pingTable.getAllPings();
Map<UUID, List<Nickname>> allNicknames = nicknamesTable.getAllNicknamesUnmapped();
Map<UUID, List<Session>> sessions = sessionsTable.getSessionInfoOfServer(serverUUID);
Map<UUID, Map<UUID, List<Session>>> map = new HashMap<>();
@ -144,7 +146,7 @@ public class SQLFetchOps extends SQLOps implements FetchOperations {
container.putRawData(PlayerKeys.KICK_COUNT, timesKicked.get(uuid));
container.putRawData(PlayerKeys.GEO_INFO, geoInfo.get(uuid));
container.putRawData(PlayerKeys.PING, allPings.get(uuid));
container.putCachingSupplier(PlayerKeys.NICKNAMES, () -> nicknamesTable.getNicknameInformation(uuid));
container.putRawData(PlayerKeys.NICKNAMES, allNicknames.get(uuid));
container.putRawData(PlayerKeys.PER_SERVER, perServerInfo.get(uuid));
container.putRawData(PlayerKeys.BANNED, userInfo.isBanned());
@ -187,6 +189,7 @@ public class SQLFetchOps extends SQLOps implements FetchOperations {
Map<UUID, Integer> timesKicked = usersTable.getAllTimesKicked();
Map<UUID, List<GeoInfo>> geoInfo = geoInfoTable.getAllGeoInfo();
Map<UUID, List<Ping>> allPings = pingTable.getAllPings();
Map<UUID, List<Nickname>> allNicknames = nicknamesTable.getAllNicknamesUnmapped();
Map<UUID, Map<UUID, List<Session>>> sessions = sessionsTable.getAllSessions(false);
Map<UUID, List<UserInfo>> allUserInfo = userInfoTable.getAllUserInfo();
@ -202,7 +205,7 @@ public class SQLFetchOps extends SQLOps implements FetchOperations {
container.putRawData(PlayerKeys.KICK_COUNT, timesKicked.get(uuid));
container.putRawData(PlayerKeys.GEO_INFO, geoInfo.get(uuid));
container.putRawData(PlayerKeys.PING, allPings.get(uuid));
container.putCachingSupplier(PlayerKeys.NICKNAMES, () -> nicknamesTable.getNicknameInformation(uuid));
container.putRawData(PlayerKeys.NICKNAMES, allNicknames.get(uuid));
container.putRawData(PlayerKeys.PER_SERVER, perServerInfo.get(uuid));
container.putCachingSupplier(PlayerKeys.SESSIONS, () -> {

View File

@ -162,6 +162,42 @@ public class NicknamesTable extends UserIDTable {
});
}
/**
* Get nicknames of all users but doesn't map them by Server
*
* @return a {@code Map<UUID, List<Nickname>} with all nicknames of all users
* @see NicknamesTable#getAllNicknames();
*/
public Map<UUID, List<Nickname>> getAllNicknamesUnmapped() {
String usersIDColumn = usersTable + "." + UsersTable.Col.ID;
String usersUUIDColumn = usersTable + "." + UsersTable.Col.UUID + " as uuid";
String serverIDColumn = serverTable + "." + ServerTable.Col.SERVER_ID;
String serverUUIDColumn = serverTable + "." + ServerTable.Col.SERVER_UUID + " as s_uuid";
String sql = "SELECT " +
Col.NICKNAME + ", " +
Col.LAST_USED + ", " +
usersUUIDColumn + ", " +
serverUUIDColumn +
" FROM " + tableName +
" INNER JOIN " + usersTable + " on " + usersIDColumn + "=" + Col.USER_ID +
" INNER JOIN " + serverTable + " on " + serverIDColumn + "=" + Col.SERVER_ID;
return query(new QueryAllStatement<Map<UUID, List<Nickname>>>(sql, 5000) {
@Override
public Map<UUID, List<Nickname>> processResults(ResultSet set) throws SQLException {
Map<UUID, List<Nickname>> map = new HashMap<>();
while (set.next()) {
UUID uuid = UUID.fromString(set.getString("uuid"));
UUID serverUUID = UUID.fromString(set.getString("s_uuid"));
List<Nickname> nicknames = map.computeIfAbsent(uuid, x -> new ArrayList<>());
nicknames.add(new Nickname(
set.getString(Col.NICKNAME.get()), set.getLong(Col.LAST_USED.get()), serverUUID
));
}
return map;
}
});
}
public void saveUserName(UUID uuid, Nickname name) {
List<Nickname> saved = getNicknameInformation(uuid);
if (saved.contains(name)) {

View File

@ -187,8 +187,7 @@ public class UserInfoTable extends UserIDTable {
return new ArrayList<>();
}
Map<UUID, String> playerNames = usersTable.getPlayerNames();
Map<Integer, UUID> uuidsByID = usersTable.getUUIDsByID();
Map<Integer, Map.Entry<UUID, String>> uuidsAndNamesByID = usersTable.getUUIDsAndNamesByID();
String sql = "SELECT * FROM " + tableName +
" WHERE " + Col.SERVER_ID + "=?";
@ -207,8 +206,11 @@ public class UserInfoTable extends UserIDTable {
boolean op = set.getBoolean(Col.OP.get());
boolean banned = set.getBoolean(Col.BANNED.get());
int userId = set.getInt(Col.USER_ID.get());
UUID uuid = uuidsByID.get(userId);
String name = playerNames.getOrDefault(uuid, "Unknown");
Map.Entry<UUID, String> uuidNameEntry = uuidsAndNamesByID.get(userId);
UUID uuid = uuidNameEntry.getKey();
String name = uuidNameEntry.getValue();
UserInfo info = new UserInfo(uuid, name, registered, op, banned);
if (!userInfo.contains(info)) {
userInfo.add(info);

View File

@ -416,6 +416,30 @@ public class UsersTable extends UserIDTable {
});
}
/**
* Gets the {@code UUID} and the name of the player mapped to the user ID
*
* @return a {@code Map<Integer, Map.Entry<UUID, String>>} where the key is the user ID
* and the value is an {@code Map.Entry<UUID, String>>} of the player's {@code UUID} and name
*/
public Map<Integer, Map.Entry<UUID, String>> getUUIDsAndNamesByID() {
String sql = Select.from(tableName, Col.ID, Col.UUID, Col.USER_NAME).toString();
return query(new QueryAllStatement<Map<Integer, Map.Entry<UUID, String>>>(sql, 20000) {
@Override
public Map<Integer, Map.Entry<UUID, String>> processResults(ResultSet set) throws SQLException {
Map<Integer, Map.Entry<UUID, String>> uuidsAndNamesByID = new TreeMap<>();
while (set.next()) {
int id = set.getInt(Col.ID.get());
UUID uuid = UUID.fromString(set.getString(Col.UUID.get()));
String name = set.getString(Col.USER_NAME.get());
uuidsAndNamesByID.put(id, new AbstractMap.SimpleEntry<>(uuid, name));
}
return uuidsAndNamesByID;
}
});
}
public DataContainer getUserInformation(UUID uuid) {
Key<DataContainer> user_data = new Key<>(DataContainer.class, "plan_users_data");
DataContainer returnValue = new DataContainer();

View File

@ -126,7 +126,7 @@ public abstract class CommonDBTest {
db.commit(db.getConnection());
}
@Test(timeout = 3000)
@Test
public void testSaveCommandUse() throws DBInitException {
CommandUseTable commandUseTable = db.getCommandUseTable();
Map<String, Integer> expected = new HashMap<>();

View File

@ -42,12 +42,12 @@ public class SQLiteTest extends CommonDBTest {
}
@Test
public void testH2GetConfigName() {
public void testSQLiteGetConfigName() {
assertEquals("sqlite", db.getType().getConfigName());
}
@Test
public void testH2GetName() {
public void testSQLiteGetName() {
assertEquals("SQLite", db.getType().getName());
}

View File

@ -14,7 +14,7 @@ import com.djrapitops.plugin.logging.debug.DebugLogger;
import com.djrapitops.plugin.logging.debug.MemoryDebugLogger;
import com.djrapitops.plugin.logging.error.ConsoleErrorLogger;
import com.djrapitops.plugin.logging.error.ErrorHandler;
import com.djrapitops.plugin.task.thread.ThreadRunnableFactory;
import com.djrapitops.plugin.task.RunnableFactory;
import org.bukkit.Server;
import org.bukkit.command.ConsoleCommandSender;
import org.bukkit.plugin.InvalidDescriptionException;
@ -23,6 +23,7 @@ import org.bukkit.scheduler.BukkitScheduler;
import org.mockito.Mockito;
import utilities.TestConstants;
import utilities.mocks.objects.TestLogger;
import utilities.mocks.objects.TestRunnableFactory;
import java.io.File;
import java.io.FileInputStream;
@ -54,7 +55,7 @@ public class PlanBukkitMocker extends Mocker {
doReturn("1.0.0").when(planMock).getVersion();
TestLogger testLogger = new TestLogger();
ThreadRunnableFactory runnableFactory = new ThreadRunnableFactory();
RunnableFactory runnableFactory = new TestRunnableFactory();
PluginLogger testPluginLogger = new TestPluginLogger();
DebugLogger debugLogger = new CombineDebugLogger(new MemoryDebugLogger());
ErrorHandler consoleErrorLogger = new ConsoleErrorLogger(testPluginLogger);

View File

@ -0,0 +1,113 @@
/*
* License is provided in the jar as LICENSE also here:
* https://github.com/Rsl1122/Plan-PlayerAnalytics/blob/master/Plan/src/main/resources/LICENSE
*/
package utilities.mocks.objects;
import com.djrapitops.plugin.api.TimeAmount;
import com.djrapitops.plugin.task.AbsRunnable;
import com.djrapitops.plugin.task.PluginRunnable;
import com.djrapitops.plugin.task.PluginTask;
import com.djrapitops.plugin.task.RunnableFactory;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
/**
* @author Fuzzlemann
* @since 4.5.1
*/
public class TestRunnableFactory extends RunnableFactory {
private final ScheduledExecutorService executorService = Executors.newScheduledThreadPool(1);
@Override
protected PluginRunnable createNewRunnable(String name, AbsRunnable absRunnable, long l) {
return new PluginRunnable() {
@Override
public String getTaskName() {
return name;
}
@Override
public void cancel() {
absRunnable.cancel();
}
@Override
public int getTaskId() {
return absRunnable.getTaskId();
}
@Override
public PluginTask runTask() {
absRunnable.run();
return createPluginTask(getTaskId(), true, absRunnable::cancel);
}
@Override
public PluginTask runTaskAsynchronously() {
executorService.submit(absRunnable);
return createPluginTask(getTaskId(), false, absRunnable::cancel);
}
@Override
public PluginTask runTaskLater(long l) {
return runTaskLaterAsynchronously(l);
}
@Override
public PluginTask runTaskLaterAsynchronously(long l) {
executorService.schedule(absRunnable, TimeAmount.ticksToMillis(l), TimeUnit.MILLISECONDS);
return createPluginTask(getTaskId(), false, absRunnable::cancel);
}
@Override
public PluginTask runTaskTimer(long l, long l1) {
return runTaskLaterAsynchronously(l);
}
@Override
public PluginTask runTaskTimerAsynchronously(long l, long l1) {
executorService.scheduleAtFixedRate(absRunnable, TimeAmount.ticksToMillis(l), TimeAmount.ticksToMillis(l1), TimeUnit.MILLISECONDS);
return createPluginTask(getTaskId(), false, absRunnable::cancel);
}
@Override
public long getTime() {
return l;
}
};
}
@Override
public void cancelAllKnownTasks() {
executorService.shutdownNow();
}
private PluginTask createPluginTask(int taskID, boolean sync, ICloseTask closeTask) {
return new PluginTask() {
@Override
public int getTaskId() {
return taskID;
}
@Override
public boolean isSync() {
return sync;
}
@Override
public void cancel() {
if (closeTask != null) {
closeTask.close();
}
}
};
}
private interface ICloseTask {
void close();
}
}