From e04c24a65a09b2f2d0ef3cc63fcc054ef7801bec Mon Sep 17 00:00:00 2001 From: Archy-X <63976867+Archy-X@users.noreply.github.com> Date: Sat, 28 Dec 2024 15:24:07 -0700 Subject: [PATCH] Add api methods to load and save offline users - Add more advanced locks to protect against simultaneous loads and saves - Update stats on join on the main thread to fix errors --- .../auraskills/api/user/SkillsUser.java | 11 +++ .../auraskills/api/user/UserManager.java | 12 +++ .../bukkit/commands/SkillsRootCommand.java | 2 +- .../bukkit/listeners/PlayerJoinQuit.java | 2 +- .../api/implementation/ApiSkillsUser.java | 19 +++++ .../api/implementation/ApiUserManager.java | 26 +++++++ .../api/implementation/OfflineSkillsUser.java | 8 ++ .../common/storage/StorageProvider.java | 75 +++++++++++++++---- .../common/storage/backup/BackupProvider.java | 2 +- .../storage/file/FileStorageProvider.java | 4 - .../aurelium/auraskills/common/user/User.java | 10 --- 11 files changed, 140 insertions(+), 31 deletions(-) diff --git a/api/src/main/java/dev/aurelium/auraskills/api/user/SkillsUser.java b/api/src/main/java/dev/aurelium/auraskills/api/user/SkillsUser.java index fd0a77e41..8bbf3f7e6 100644 --- a/api/src/main/java/dev/aurelium/auraskills/api/user/SkillsUser.java +++ b/api/src/main/java/dev/aurelium/auraskills/api/user/SkillsUser.java @@ -14,6 +14,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; public interface SkillsUser { @@ -336,4 +337,14 @@ public interface SkillsUser { */ void pauseActionBar(int duration, TimeUnit timeUnit); + /** + * Saves the user to persistent storage asynchronously. This is only recommended if the SkillsUser represents an + * offline user, as changes for online users are saved automatically on logout. + * + * @param removeFromMemory Whether to remove the user from memory after saving. Will not work if the user is online. + * @return A future signaling when the saving is complete. The future's value is true if successful, and false if an + * exception was thrown. + */ + CompletableFuture save(boolean removeFromMemory); + } diff --git a/api/src/main/java/dev/aurelium/auraskills/api/user/UserManager.java b/api/src/main/java/dev/aurelium/auraskills/api/user/UserManager.java index 90315a421..edc40c909 100644 --- a/api/src/main/java/dev/aurelium/auraskills/api/user/UserManager.java +++ b/api/src/main/java/dev/aurelium/auraskills/api/user/UserManager.java @@ -1,6 +1,7 @@ package dev.aurelium.auraskills.api.user; import java.util.UUID; +import java.util.concurrent.CompletableFuture; public interface UserManager { @@ -17,4 +18,15 @@ public interface UserManager { */ SkillsUser getUser(UUID playerId); + /** + * Loads an offline user from storage, or gets a user from memory if the player is online. When loading offline users, + * the returned {@link SkillsUser} may become out of date at any point if the user logs in, since joining will always + * load from storage. If the UUID doesn't represent a user that exists in either memory or storage, an empty + * {@link SkillsUser} object will be returned where {@link SkillsUser#isLoaded()} will return false. + * + * @param playerId the UUID of the player + * @return a future with the SkillsUser, which is provided after the user is loaded asynchronously + */ + CompletableFuture loadUser(UUID playerId); + } diff --git a/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/commands/SkillsRootCommand.java b/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/commands/SkillsRootCommand.java index 81cde2574..d7be632a9 100644 --- a/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/commands/SkillsRootCommand.java +++ b/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/commands/SkillsRootCommand.java @@ -174,7 +174,7 @@ public void onSave(CommandSender sender) { Locale locale = plugin.getLocale(sender); for (User user : plugin.getUserManager().getOnlineUsers()) { try { - plugin.getStorageProvider().save(user); + plugin.getStorageProvider().saveSafely(user); } catch (Exception e) { e.printStackTrace(); } diff --git a/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/listeners/PlayerJoinQuit.java b/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/listeners/PlayerJoinQuit.java index 19a4ce818..44a479af6 100644 --- a/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/listeners/PlayerJoinQuit.java +++ b/bukkit/src/main/java/dev/aurelium/auraskills/bukkit/listeners/PlayerJoinQuit.java @@ -57,7 +57,7 @@ public void onQuit(PlayerQuitEvent event) { plugin.getScheduler().executeAsync(() -> { try { - plugin.getStorageProvider().save(user); + plugin.getStorageProvider().saveSafely(user); plugin.getUserManager().removeUser(player.getUniqueId()); } catch (Exception e) { e.printStackTrace(); diff --git a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiSkillsUser.java b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiSkillsUser.java index 501aab2b2..b55194075 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiSkillsUser.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiSkillsUser.java @@ -19,6 +19,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; public class ApiSkillsUser implements SkillsUser { @@ -255,4 +256,22 @@ public void pauseActionBar(int duration, TimeUnit timeUnit) { plugin.getUiProvider().getActionBarManager().setPaused(user, duration, timeUnit); } + @Override + public CompletableFuture save(boolean removeFromMemory) { + CompletableFuture future = new CompletableFuture<>(); + plugin.getScheduler().executeAsync(() -> { + try { + plugin.getStorageProvider().saveSafely(user); + if (removeFromMemory) { + plugin.getUserManager().removeUser(user.getUuid()); + } + future.complete(true); + } catch (Exception e) { + e.printStackTrace(); + future.complete(false); + } + }); + return future; + } + } diff --git a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiUserManager.java b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiUserManager.java index f34093b46..e6921f874 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiUserManager.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/ApiUserManager.java @@ -6,6 +6,7 @@ import dev.aurelium.auraskills.common.user.User; import java.util.UUID; +import java.util.concurrent.CompletableFuture; public class ApiUserManager implements UserManager { @@ -24,4 +25,29 @@ public SkillsUser getUser(UUID playerId) { return new OfflineSkillsUser(plugin, playerId); } + @Override + public CompletableFuture loadUser(UUID playerId) { + CompletableFuture future = new CompletableFuture<>(); + User onlineUser = plugin.getUserManager().getUser(playerId); + if (onlineUser != null) { + future.complete(new ApiSkillsUser(onlineUser)); + } else { + plugin.getScheduler().executeAsync(() -> { + try { + plugin.getStorageProvider().load(playerId); + User loadedUser = plugin.getUserManager().getUser(playerId); + if (loadedUser != null) { + future.complete(new ApiSkillsUser(loadedUser)); + } else { + future.complete(new OfflineSkillsUser(plugin, playerId)); + } + } catch (Exception e) { + future.complete(new OfflineSkillsUser(plugin, playerId)); + e.printStackTrace(); + } + }); + } + return future; + } + } diff --git a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/OfflineSkillsUser.java b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/OfflineSkillsUser.java index 0f442137a..9744fac3b 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/OfflineSkillsUser.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/api/implementation/OfflineSkillsUser.java @@ -13,6 +13,7 @@ import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; public class OfflineSkillsUser implements SkillsUser { @@ -224,4 +225,11 @@ public void sendActionBar(String message, int duration, TimeUnit timeUnit) { public void pauseActionBar(int duration, TimeUnit timeUnit) { } + + @Override + public CompletableFuture save(boolean removeFromMemory) { + var future = new CompletableFuture(); + future.complete(false); + return future; + } } diff --git a/common/src/main/java/dev/aurelium/auraskills/common/storage/StorageProvider.java b/common/src/main/java/dev/aurelium/auraskills/common/storage/StorageProvider.java index 57c610fe5..18aa6d0fd 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/storage/StorageProvider.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/storage/StorageProvider.java @@ -12,12 +12,18 @@ import java.util.List; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantReadWriteLock; public abstract class StorageProvider { + private static final long SAVE_TIMEOUT_MS = 2000; + private static final long LOAD_TIMEOUT_MS = 2000; + public final AuraSkillsPlugin plugin; public final UserManager userManager; + private final ConcurrentHashMap userLocks = new ConcurrentHashMap<>(); public StorageProvider(AuraSkillsPlugin plugin) { this.userManager = plugin.getUserManager(); @@ -25,19 +31,34 @@ public StorageProvider(AuraSkillsPlugin plugin) { } public void load(UUID uuid) throws Exception { - User user = loadRaw(uuid); - fixInvalidData(user); + ReentrantReadWriteLock lock = getUserLock(uuid); + boolean lockAcquired = false; + try { + lockAcquired = lock.readLock().tryLock(LOAD_TIMEOUT_MS, TimeUnit.MILLISECONDS); + if (!lockAcquired) { + plugin.logger().warn("Load timout exceeded for user " + uuid); + } - plugin.getUserManager().addUser(user); + User user = loadRaw(uuid); + fixInvalidData(user); - // Update stats - plugin.getStatManager().updateStats(user); + plugin.getUserManager().addUser(user); - // Call event - plugin.getScheduler().executeSync(() -> plugin.getEventHandler().callUserLoadEvent(user)); + plugin.getScheduler().executeSync(() -> { + plugin.getStatManager().updateStats(user); // Update stats + plugin.getEventHandler().callUserLoadEvent(user); // Call event + }); - // Update permissions - plugin.getRewardManager().updatePermissions(user); + // Update permissions + plugin.getRewardManager().updatePermissions(user); + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + if (lockAcquired) { + lock.readLock().unlock(); + } + removeUserLock(uuid, lock); + } } protected abstract User loadRaw(UUID uuid) throws Exception; @@ -59,6 +80,26 @@ public void load(UUID uuid) throws Exception { */ public abstract void applyState(UserState state) throws Exception; + public void saveSafely(@NotNull User user) { + ReentrantReadWriteLock lock = getUserLock(user.getUuid()); + boolean lockAcquired = false; + try { + lockAcquired = lock.writeLock().tryLock(SAVE_TIMEOUT_MS, TimeUnit.MILLISECONDS); + if (!lockAcquired) { + plugin.logger().warn("Save timeout exceeded for user " + user.getUuid()); + return; + } + save(user); + } catch (Exception e) { + e.printStackTrace(); + } finally { + if (lockAcquired) { + lock.writeLock().unlock(); + } + removeUserLock(user.getUuid(), lock); + } + } + public abstract void save(@NotNull User user) throws Exception; public abstract void delete(UUID uuid) throws Exception; @@ -77,12 +118,8 @@ public void startAutoSaving() { public void run() { for (User user : userManager.getOnlineUsers()) { try { - if (user.isSaving()) { - continue; - } - save(user); + saveSafely(user); } catch (Exception e) { - user.setSaving(false); plugin.logger().warn("Error running auto-save on user data:"); e.printStackTrace(); } @@ -111,4 +148,14 @@ private void fixInvalidData(User user) { } } + protected ReentrantReadWriteLock getUserLock(UUID uuid) { + return userLocks.computeIfAbsent(uuid, id -> new ReentrantReadWriteLock()); + } + + protected void removeUserLock(UUID uuid, ReentrantReadWriteLock lock) { + if (lock.getReadLockCount() == 0 && !lock.isWriteLocked()) { + userLocks.remove(uuid); + } + } + } diff --git a/common/src/main/java/dev/aurelium/auraskills/common/storage/backup/BackupProvider.java b/common/src/main/java/dev/aurelium/auraskills/common/storage/backup/BackupProvider.java index 522258072..769797119 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/storage/backup/BackupProvider.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/storage/backup/BackupProvider.java @@ -44,7 +44,7 @@ public File saveBackup(boolean savePlayerData) throws Exception { // Save online players if (savePlayerData) { for (User user : plugin.getUserManager().getOnlineUsers()) { - plugin.getStorageProvider().save(user); + plugin.getStorageProvider().saveSafely(user); } } diff --git a/common/src/main/java/dev/aurelium/auraskills/common/storage/file/FileStorageProvider.java b/common/src/main/java/dev/aurelium/auraskills/common/storage/file/FileStorageProvider.java index a1b134e45..78124c060 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/storage/file/FileStorageProvider.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/storage/file/FileStorageProvider.java @@ -285,9 +285,6 @@ public void applyState(UserState state) throws Exception { @Override public void save(@NotNull User user) throws Exception { - if (user.isSaving()) return; - user.setSaving(true); - CommentedConfigurationNode root = loadYamlFile(user.getUuid()); root.node("uuid").set(user.getUuid().toString()); @@ -386,7 +383,6 @@ public void save(@NotNull User user) throws Exception { } saveYamlFile(root, user.getUuid()); - user.setSaving(false); } private void saveYamlFile(CommentedConfigurationNode root, UUID uuid) throws ConfigurateException { diff --git a/common/src/main/java/dev/aurelium/auraskills/common/user/User.java b/common/src/main/java/dev/aurelium/auraskills/common/user/User.java index d4787e108..ba2025606 100644 --- a/common/src/main/java/dev/aurelium/auraskills/common/user/User.java +++ b/common/src/main/java/dev/aurelium/auraskills/common/user/User.java @@ -54,7 +54,6 @@ public abstract class User { private long lastJobSelectTime; private final List sessionAntiAfkLogs; - private boolean saving; private boolean shouldSave; private boolean blank = true; @@ -78,7 +77,6 @@ public User(UUID uuid, AuraSkillsPlugin plugin) { this.metadata = new ConcurrentHashMap<>(); this.actionBarSettings = new ConcurrentHashMap<>(); this.unclaimedItems = new LinkedList<>(); - this.saving = false; this.shouldSave = true; this.mana = Traits.MAX_MANA.isEnabled() ? Traits.MAX_MANA.optionDouble("base") : 0.0; this.multipliers = new HashMap<>(); @@ -464,14 +462,6 @@ public int getJobLimit() { public abstract boolean canSelectJob(@NotNull Skill skill); - public boolean isSaving() { - return saving; - } - - public void setSaving(boolean saving) { - this.saving = saving; - } - public boolean shouldNotSave() { return !shouldSave; }