From 7e18989fb7e6e75b3e75198fa1fef4bfb3f67e83 Mon Sep 17 00:00:00 2001 From: samczsun Date: Sun, 12 Feb 2017 19:59:31 -0500 Subject: [PATCH] PC-267 PC-545 PC-1253 This is to fix a troublesome race condition occurring between Mineplexer and Core. Simply put, the following steps occur normally: 1) Player logs in via Bungee 2) If there is no PlayerInfo entry, PlayerStats inserts a PlayerInfo entry into Redis 3) CoreClientManager looks for the PlayerInfo entry and finds it 4) If the PlayerInfo entry was just inserted, it will have accountId=0. CoreClientManager sees this and updates it to a valid accountId, then reinserts into Redis 5) All is good However, sometimes Step 3 occurs before Step 2 (perhaps latency to Redis is a factor), and so CoreClientManager sees a null entry and ignores it. Then, an invalid PlayerInfo entry is inserted with accountId=0, which then breaks any SQL queries relying on an valid accountId --- .../mineplex/cache/player/PlayerCache.java | 76 +++++++++++++------ .../src/mineplex/cache/player/PlayerInfo.java | 10 --- .../core/account/CoreClientManager.java | 26 +------ .../account/repository/AccountRepository.java | 2 - .../core/cosmetic/ui/page/PetTagPage.java | 2 +- .../core/inventory/InventoryManager.java | 7 +- .../core/reward/rewards/PetReward.java | 2 +- .../src/mineplex/core/task/TaskManager.java | 51 +++++++------ .../redis/atomic/RedisStringRepository.java | 59 ++++++++++++++ 9 files changed, 148 insertions(+), 87 deletions(-) create mode 100644 Plugins/Mineplex.ServerData/src/mineplex/serverdata/redis/atomic/RedisStringRepository.java diff --git a/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerCache.java b/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerCache.java index 804515ad2..2409e3be7 100644 --- a/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerCache.java +++ b/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerCache.java @@ -4,37 +4,43 @@ import java.util.UUID; import mineplex.serverdata.Region; import mineplex.serverdata.redis.RedisDataRepository; +import mineplex.serverdata.redis.atomic.RedisStringRepository; import mineplex.serverdata.servers.ServerManager; -public class PlayerCache +public enum PlayerCache { - private static PlayerCache _instance = null; - - private RedisDataRepository _repository; + INSTANCE; public static PlayerCache getInstance() { - if (_instance == null) - _instance = new PlayerCache(); - - return _instance; + return INSTANCE; } - - private PlayerCache() + + private final RedisDataRepository _playerInfoRepository; + private final RedisStringRepository _accountIdRepository; + + PlayerCache() { - _repository = new RedisDataRepository( - ServerManager.getMasterConnection(), + _playerInfoRepository = new RedisDataRepository( + ServerManager.getMasterConnection(), ServerManager.getSlaveConnection(), - Region.ALL, - PlayerInfo.class, + Region.ALL, + PlayerInfo.class, "playercache"); + + _accountIdRepository = new RedisStringRepository( + ServerManager.getMasterConnection(), + ServerManager.getSlaveConnection(), + Region.ALL, + "accountid" + ); } - + public void addPlayer(PlayerInfo player) { try { - _repository.addElement(player, 60 * 60 * 6); // 6 Hours + _playerInfoRepository.addElement(player, 60 * 60 * 6); // 6 Hours } catch (Exception exception) { @@ -42,12 +48,12 @@ public class PlayerCache exception.printStackTrace(); } } - + public PlayerInfo getPlayer(UUID uuid) { try { - PlayerInfo playerInfo = _repository.getElement(uuid.toString()); + PlayerInfo playerInfo = _playerInfoRepository.getElement(uuid.toString()); return playerInfo; } catch (Exception exception) @@ -55,23 +61,49 @@ public class PlayerCache System.out.println("Error retrieving player info in PlayerCache : " + exception.getMessage()); exception.printStackTrace(); } - + return null; } /** * Attempts to grab a player's account ID from the cache + * * @param uuid Minecraft Account UUID * @return The account id of the player, or -1 if the player is not in the cache */ public int getAccountId(UUID uuid) { - PlayerInfo info = getPlayer(uuid); - return info == null ? -1 : info.getAccountId(); + String accountIdStr = _accountIdRepository.get(uuid.toString()); + + if (accountIdStr == null) + return -1; + + try + { + int accountId = Integer.parseInt(accountIdStr); + if (accountId <= 0) + { + // remove invalid account id + _accountIdRepository.del(uuid.toString()); + return -1; + } + return accountId; + } + catch (NumberFormatException ex) + { + // remove invalid account id + _accountIdRepository.del(uuid.toString()); + return -1; + } + } + + public void updateAccountId(UUID uuid, int newId) + { + _accountIdRepository.set(uuid.toString(), String.valueOf(newId)); } public void clean() { - _repository.clean(); + _playerInfoRepository.clean(); } } diff --git a/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerInfo.java b/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerInfo.java index c7cf6faec..5719f661b 100644 --- a/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerInfo.java +++ b/Plugins/Mineplex.Cache/src/mineplex/cache/player/PlayerInfo.java @@ -36,11 +36,6 @@ public class PlayerInfo implements Data return _id; } - public int getAccountId() - { - return _accountId; - } - public UUID getUUID() { return _uuid; @@ -90,11 +85,6 @@ public class PlayerInfo implements Data { _version = version; } - - public void setAccountId(int accountId) - { - _accountId = accountId; - } public void updateLoginTime() { diff --git a/Plugins/Mineplex.Core/src/mineplex/core/account/CoreClientManager.java b/Plugins/Mineplex.Core/src/mineplex/core/account/CoreClientManager.java index 79bccee81..37ca50942 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/account/CoreClientManager.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/account/CoreClientManager.java @@ -31,7 +31,6 @@ import com.google.common.collect.Sets; import com.google.gson.Gson; import mineplex.cache.player.PlayerCache; -import mineplex.cache.player.PlayerInfo; import mineplex.core.MiniPlugin; import mineplex.core.account.command.TestRank; import mineplex.core.account.command.UpdateRank; @@ -301,13 +300,7 @@ public class CoreClientManager extends MiniPlugin if (client.getAccountId() > 0) { - PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid); - - if (playerInfo != null) - { - playerInfo.setAccountId(client.getAccountId()); - PlayerCache.getInstance().addPlayer(playerInfo); - } + PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId()); } loaded.set(client); @@ -367,13 +360,7 @@ public class CoreClientManager extends MiniPlugin if (client.getAccountId() > 0) { - PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid); - - if (playerInfo != null) - { - playerInfo.setAccountId(client.getAccountId()); - PlayerCache.getInstance().addPlayer(playerInfo); - } + PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId()); } } catch (Exception exception) @@ -458,14 +445,7 @@ public class CoreClientManager extends MiniPlugin if (client.getAccountId() > 0) { - PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid); - - if (playerInfo != null) - { - client.setNetworkSessionLoginTime(playerInfo.getLoginTime()); - playerInfo.setAccountId(client.getAccountId()); - PlayerCache.getInstance().addPlayer(playerInfo); - } + PlayerCache.getInstance().updateAccountId(uuid, client.getAccountId()); } return !CLIENT_LOGIN_LOCKS.containsKey(client.getName()); diff --git a/Plugins/Mineplex.Core/src/mineplex/core/account/repository/AccountRepository.java b/Plugins/Mineplex.Core/src/mineplex/core/account/repository/AccountRepository.java index ac5d9bba0..789914489 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/account/repository/AccountRepository.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/account/repository/AccountRepository.java @@ -11,8 +11,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import org.apache.commons.dbcp2.BasicDataSource; -import org.bukkit.Bukkit; -import org.bukkit.plugin.java.JavaPlugin; import com.google.gson.reflect.TypeToken; diff --git a/Plugins/Mineplex.Core/src/mineplex/core/cosmetic/ui/page/PetTagPage.java b/Plugins/Mineplex.Core/src/mineplex/core/cosmetic/ui/page/PetTagPage.java index 49756c42f..e81295364 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/cosmetic/ui/page/PetTagPage.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/cosmetic/ui/page/PetTagPage.java @@ -110,7 +110,7 @@ public class PetTagPage extends ShopPageBase if (getClientManager().Get(getPlayer()) != null) token.AccountId = getClientManager().Get(getPlayer()).getAccountId(); else - token.AccountId = PlayerCache.getInstance().getPlayer(getPlayer().getUniqueId()).getAccountId(); + token.AccountId = PlayerCache.getInstance().getAccountId(getPlayer().getUniqueId()); token.Name = getPlayer().getName(); token.PetType = _petType.toString(); diff --git a/Plugins/Mineplex.Core/src/mineplex/core/inventory/InventoryManager.java b/Plugins/Mineplex.Core/src/mineplex/core/inventory/InventoryManager.java index 32d51fd55..88342d0c4 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/inventory/InventoryManager.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/inventory/InventoryManager.java @@ -1,7 +1,6 @@ package mineplex.core.inventory; import mineplex.cache.player.PlayerCache; -import mineplex.cache.player.PlayerInfo; import mineplex.core.MiniDbClientPlugin; import mineplex.core.account.CoreClientManager; import mineplex.core.common.util.Callback; @@ -144,10 +143,10 @@ public class InventoryManager extends MiniDbClientPlugin { public void run() { - PlayerInfo playerInfo = PlayerCache.getInstance().getPlayer(uuid); - if (playerInfo != null) + int accountId = PlayerCache.getInstance().getAccountId(uuid); + if (accountId != -1) { - addItemToInventoryForOffline(callback, playerInfo.getAccountId(), item, count); + addItemToInventoryForOffline(callback, accountId, item, count); } else { diff --git a/Plugins/Mineplex.Core/src/mineplex/core/reward/rewards/PetReward.java b/Plugins/Mineplex.Core/src/mineplex/core/reward/rewards/PetReward.java index 19f500c87..23e4e179a 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/reward/rewards/PetReward.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/reward/rewards/PetReward.java @@ -41,7 +41,7 @@ public class PetReward extends UnknownPackageReward if (_inventoryManager.getClientManager().Get(player) != null) token.AccountId = _inventoryManager.getClientManager().Get(player).getAccountId(); else - token.AccountId = PlayerCache.getInstance().getPlayer(player.getUniqueId()).getAccountId(); + token.AccountId = PlayerCache.getInstance().getAccountId(player.getUniqueId()); token.Name = player.getName(); token.PetType = _petType.toString(); diff --git a/Plugins/Mineplex.Core/src/mineplex/core/task/TaskManager.java b/Plugins/Mineplex.Core/src/mineplex/core/task/TaskManager.java index 2d7d41fc3..9f73c6365 100644 --- a/Plugins/Mineplex.Core/src/mineplex/core/task/TaskManager.java +++ b/Plugins/Mineplex.Core/src/mineplex/core/task/TaskManager.java @@ -4,12 +4,14 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.List; import java.util.UUID; +import java.util.function.Consumer; import mineplex.cache.player.PlayerCache; import mineplex.core.MiniDbClientPlugin; import mineplex.core.account.CoreClientManager; import mineplex.core.common.util.Callback; import mineplex.core.common.util.NautHashMap; +import mineplex.core.common.util.UtilTasks; import mineplex.core.task.repository.TaskRepository; import org.bukkit.Bukkit; @@ -54,7 +56,7 @@ public class TaskManager extends MiniDbClientPlugin return new TaskClient(); } - public void addTaskForOfflinePlayer(final Callback callback, final UUID uuid, final String task) + public void addTaskForOfflinePlayer(Consumer callback, final UUID uuid, final String task) { Bukkit.getServer().getScheduler().runTaskAsynchronously(getPlugin(), new Runnable() { @@ -75,16 +77,20 @@ public class TaskManager extends MiniDbClientPlugin updateTasks(); } - final boolean success = _repository.addAccountTask(PlayerCache.getInstance().getPlayer(uuid).getAccountId(), getTaskId(task)); - - if (callback != null) + int accountId = PlayerCache.getInstance().getAccountId(uuid); + + if (accountId != -1) { - Bukkit.getServer().getScheduler().runTask(getPlugin(), new Runnable() + UtilTasks.onMainThread(callback).accept(_repository.addAccountTask(accountId, getTaskId(task))); + } + else + { + ClientManager.loadAccountIdFromUUID(uuid, id -> { - public void run() - { - callback.run(success); - } + if (id > 0) + UtilTasks.onMainThread(callback).accept(_repository.addAccountTask(accountId, getTaskId(task))); + else + UtilTasks.onMainThread(callback).accept(false); }); } } @@ -114,27 +120,24 @@ public class TaskManager extends MiniDbClientPlugin } } - addTaskForOfflinePlayer(new Callback() + addTaskForOfflinePlayer(success -> { - public void run(Boolean success) + if (!success) { - if (!success.booleanValue()) + System.out.println("Add task FAILED for " + player.getName()); + + synchronized (_taskLock) { - System.out.println("Add task FAILED for " + player.getName()); - - synchronized (_taskLock) + if (_tasks.containsKey(taskName)) { - if (_tasks.containsKey(taskName)) - { - Get(player).TasksCompleted.remove(_tasks.get(taskName)); - } + Get(player).TasksCompleted.remove(_tasks.get(taskName)); } } - - if (callback != null) - { - callback.run(success); - } + } + + if (callback != null) + { + callback.run(success); } }, player.getUniqueId(), taskName); } diff --git a/Plugins/Mineplex.ServerData/src/mineplex/serverdata/redis/atomic/RedisStringRepository.java b/Plugins/Mineplex.ServerData/src/mineplex/serverdata/redis/atomic/RedisStringRepository.java new file mode 100644 index 000000000..910d61b38 --- /dev/null +++ b/Plugins/Mineplex.ServerData/src/mineplex/serverdata/redis/atomic/RedisStringRepository.java @@ -0,0 +1,59 @@ +package mineplex.serverdata.redis.atomic; + +import mineplex.serverdata.Region; +import mineplex.serverdata.redis.RedisRepository; +import mineplex.serverdata.servers.ConnectionData; + +import redis.clients.jedis.Jedis; +import redis.clients.jedis.Response; +import redis.clients.jedis.Transaction; +import static mineplex.serverdata.Utility.currentTimeMillis; + +public class RedisStringRepository extends RedisRepository +{ + private final String _dataKey; + + public RedisStringRepository(ConnectionData writeConn, ConnectionData readConn, Region region, String dataKey) + { + super(writeConn, readConn, region); + this._dataKey = dataKey; + } + + public void set(String key, String value) + { + try (Jedis jedis = getResource(true)) + { + jedis.set(generateKey(key), value); + } + } + + public String get(String key) + { + String element; + + try (Jedis jedis = getResource(false)) + { + element = jedis.get(generateKey(key)); + } + + return element; + } + + public void del(String key) + { + try (Jedis jedis = getResource(true)) + { + jedis.del(generateKey(key)); + } + } + + private String getElementSetKey() + { + return concatenate("data", _dataKey, getRegion().toString()); + } + + private String generateKey(String dataId) + { + return concatenate(getElementSetKey(), dataId); + } +}