Better handle failure conditions for background region saving.

src/main/java/com/sk89q/worldguard/bukkit/RegionContainer.java

@@ -40,6 +40,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -239,6 +240,15 @@ public List<RegionManager> getLoaded() {
         return Collections.unmodifiableList(container.getLoaded());
     }
 
+    /**
+     * Get the a set of region managers that are failing to save.
+     *
+     * @return a set of region managers
+     */
+    public Set<RegionManager> getSaveFailures() {
+        return container.getSaveFailures();
+    }
+
     /**
      * Create a new region query.
      *

src/main/java/com/sk89q/worldguard/bukkit/commands/region/MemberCommands.java

@@ -52,6 +52,8 @@ public MemberCommands(WorldGuardPlugin plugin) {
             desc = "Add a member to a region",
             min = 2)
     public void addMember(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         String id = args.getString(0);
         RegionManager manager = checkRegionManager(plugin, world);
@@ -87,6 +89,8 @@ public void addMember(CommandContext args, CommandSender sender) throws CommandE
             desc = "Add an owner to a region",
             min = 2)
     public void addOwner(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
 
         Player player = null;
@@ -148,6 +152,8 @@ public void addOwner(CommandContext args, CommandSender sender) throws CommandEx
             desc = "Remove an owner to a region",
             min = 1)
     public void removeMember(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         String id = args.getString(0);
         RegionManager manager = checkRegionManager(plugin, world);
@@ -193,6 +199,8 @@ public void removeMember(CommandContext args, CommandSender sender) throws Comma
             desc = "Remove an owner to a region",
             min = 1)
     public void removeOwner(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         String id = args.getString(0);
         RegionManager manager = checkRegionManager(plugin, world);

src/main/java/com/sk89q/worldguard/bukkit/commands/region/RegionCommands.java

@@ -98,6 +98,7 @@ public RegionCommands(WorldGuardPlugin plugin) {
              desc = "Defines a region",
              min = 1)
     public void define(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
         Player player = plugin.checkPlayer(sender);
 
         // Check permissions
@@ -140,6 +141,8 @@ public void define(CommandContext args, CommandSender sender) throws CommandExce
              desc = "Re-defines the shape of a region",
              min = 1, max = 1)
     public void redefine(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         Player player = plugin.checkPlayer(sender);
         World world = player.getWorld();
 
@@ -186,6 +189,8 @@ public void redefine(CommandContext args, CommandSender sender) throws CommandEx
              desc = "Claim a region",
              min = 1)
     public void claim(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         Player player = plugin.checkPlayer(sender);
         LocalPlayer localPlayer = plugin.wrapPlayer(player);
         RegionPermissionModel permModel = getPermissionModel(sender);
@@ -309,6 +314,8 @@ public void select(CommandContext args, CommandSender sender) throws CommandExce
              desc = "Get information about a region",
              min = 0, max = 1)
     public void info(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         RegionPermissionModel permModel = getPermissionModel(sender);
 
@@ -372,6 +379,8 @@ public void info(CommandContext args, CommandSender sender) throws CommandExcept
              flags = "np:w:",
              max = 1)
     public void list(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         String ownedBy;
         
@@ -425,6 +434,8 @@ public void list(CommandContext args, CommandSender sender) throws CommandExcept
              desc = "Set flags",
              min = 2)
     public void flag(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         String flagName = args.getString(1);
         String value = args.argsLength() >= 3 ? args.getJoinedStrings(2) : null;
@@ -563,6 +574,8 @@ public void flag(CommandContext args, CommandSender sender) throws CommandExcept
              desc = "Set the priority of a region",
              min = 2, max = 2)
     public void setPriority(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         int priority = args.getInteger(1);
 
@@ -595,6 +608,8 @@ public void setPriority(CommandContext args, CommandSender sender) throws Comman
              desc = "Set the parent of a region",
              min = 1, max = 2)
     public void setParent(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         ProtectedRegion parent;
         ProtectedRegion child;
@@ -660,6 +675,8 @@ public void setParent(CommandContext args, CommandSender sender) throws CommandE
              desc = "Remove a region",
              min = 1, max = 1)
     public void remove(CommandContext args, CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = checkWorld(args, sender, 'w'); // Get the world
         boolean removeChildren = args.hasFlag('f');
         boolean unsetParent = args.hasFlag('u');
@@ -704,6 +721,8 @@ public void remove(CommandContext args, CommandSender sender) throws CommandExce
             desc = "Reload regions from file",
             flags = "w:")
     public void load(CommandContext args, final CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = null;
         try {
             world = checkWorld(args, sender, 'w'); // Get the world
@@ -761,6 +780,8 @@ public void load(CommandContext args, final CommandSender sender) throws Command
             desc = "Re-save regions to file",
             flags = "w:")
     public void save(CommandContext args, final CommandSender sender) throws CommandException {
+        warnAboutSaveFailures(sender);
+
         World world = null;
         try {
             world = checkWorld(args, sender, 'w'); // Get the world

src/main/java/com/sk89q/worldguard/bukkit/commands/region/RegionCommandsBase.java

@@ -19,6 +19,9 @@
 
 package com.sk89q.worldguard.bukkit.commands.region;
 
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Iterables;
 import com.sk89q.minecraft.util.commands.CommandContext;
 import com.sk89q.minecraft.util.commands.CommandException;
 import com.sk89q.worldedit.BlockVector;
@@ -27,6 +30,7 @@
 import com.sk89q.worldedit.bukkit.selections.CuboidSelection;
 import com.sk89q.worldedit.bukkit.selections.Polygonal2DSelection;
 import com.sk89q.worldedit.bukkit.selections.Selection;
+import com.sk89q.worldguard.bukkit.RegionContainer;
 import com.sk89q.worldguard.bukkit.WorldGuardPlugin;
 import com.sk89q.worldguard.bukkit.permission.RegionPermissionModel;
 import com.sk89q.worldguard.protection.ApplicableRegionSet;
@@ -42,6 +46,8 @@
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 
+import java.util.Set;
+
 class RegionCommandsBase {
 
     protected RegionCommandsBase() {
@@ -274,6 +280,29 @@ protected static ProtectedRegion checkRegionFromSelection(Player player, String
         }
     }
 
+    /**
+     * Warn the region saving is failing.
+     *
+     * @param sender the sender to send the message to
+     */
+    protected static void warnAboutSaveFailures(CommandSender sender) {
+        RegionContainer container = WorldGuardPlugin.inst().getRegionContainer();
+        Set<RegionManager> failures = container.getSaveFailures();
+
+        if (failures.size() > 0) {
+            String failingList = Joiner.on(", ").join(Iterables.transform(failures, new Function<RegionManager, String>() {
+                @Override
+                public String apply(RegionManager regionManager) {
+                    return "'" + regionManager.getName() + "'";
+                }
+            }));
+
+            sender.sendMessage(ChatColor.GOLD +
+                    "(Warning: The background saving of region data is failing for these worlds: " + failingList + ". " +
+                    "Your changes are getting lost. See the server log for more information.)");
+        }
+    }
+
     /**
      * Warn the sender if the dimensions of the given region are worrying.
      *

src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardPlayerListener.java

@@ -130,6 +130,9 @@ public static boolean checkMove(WorldGuardPlugin plugin, Player player, Location
         }
 
         RegionManager mgr = plugin.getGlobalRegionManager().get(toWorld);
+        if (mgr == null) {
+            return false;
+        }
         Vector pt = new Vector(to.getBlockX(), to.getBlockY(), to.getBlockZ());
         ApplicableRegionSet set = mgr.getApplicableRegions(pt);
 

src/main/java/com/sk89q/worldguard/protection/managers/ManagerContainer.java

@@ -31,10 +31,13 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
@@ -56,6 +59,9 @@ public class ManagerContainer {
     private final Supplier<? extends ConcurrentRegionIndex> indexFactory = new ChunkHashTable.Factory(new PriorityRTreeIndex.Factory());
     private final Timer timer = new Timer();
 
+    private final Set<RegionManager> failingSaves = Collections.synchronizedSet(
+            Collections.newSetFromMap(new WeakHashMap<RegionManager, Boolean>()));
+
     /**
      * Create a new instance.
      *
@@ -188,6 +194,15 @@ public List<RegionManager> getLoaded() {
         return Collections.unmodifiableList(new ArrayList<RegionManager>(mapping.values()));
     }
 
+    /**
+     * Get the a set of region managers that are failing to save.
+     *
+     * @return a set of region managers
+     */
+    public Set<RegionManager> getSaveFailures() {
+        return new HashSet<RegionManager>(failingSaves);
+    }
+
     /**
      * A task to save managers in the background.
      */
@@ -204,9 +219,12 @@ public void run() {
                         if (manager.saveChanges()) {
                             log.info("Region data changes made in '" + name + "' have been background saved");
                         }
+                        failingSaves.remove(manager);
                     } catch (IOException e) {
+                        failingSaves.add(manager);
                         log.log(Level.WARNING, "Failed to save the region data for '" + name + "' during a periodical save", e);
                     } catch (Exception e) {
+                        failingSaves.add(manager);
                         log.log(Level.WARNING, "An expected error occurred during a periodical save", e);
                     }
                 }

src/main/java/com/sk89q/worldguard/protection/managers/RegionManager.java

@@ -69,6 +69,13 @@ public RegionManager(RegionStore store, Supplier<? extends ConcurrentRegionIndex
         this.index = indexFactory.get();
     }
 
+    /**
+     * Get a displayable name for this store.
+     */
+    public String getName() {
+        return store.getName();
+    }
+
     /**
      * Load regions from storage and replace the index on this manager with
      * the regions loaded from the store.
@@ -109,17 +116,26 @@ public void save() throws IOException {
      */
     public boolean saveChanges() throws IOException {
         RegionDifference diff = index.getAndClearDifference();
+        boolean successful = false;
 
+        try {
             if (diff.containsChanges()) {
                 try {
                     store.saveChanges(diff);
                 } catch (DifferenceSaveException e) {
                     save(); // Partial save is not supported
                 }
+                successful = true;
                 return true;
             } else {
+                successful = true;
                 return false;
             }
+        } finally {
+            if (!successful) {
+                index.setDirty(diff);
+            }
+        }
     }
 
     /**

src/main/java/com/sk89q/worldguard/protection/managers/index/ChunkHashTable.java

@@ -267,6 +267,11 @@ public RegionDifference getAndClearDifference() {
         return index.getAndClearDifference();
     }
 
+    @Override
+    public void setDirty(RegionDifference difference) {
+        index.setDirty(difference);
+    }
+
     @Override
     public Collection<ProtectedRegion> values() {
         return index.values();

src/main/java/com/sk89q/worldguard/protection/managers/index/HashMapIndex.java

@@ -236,6 +236,16 @@ public RegionDifference getAndClearDifference() {
         }
     }
 
+    @Override
+    public void setDirty(RegionDifference difference) {
+        synchronized (lock) {
+            for (ProtectedRegion changed : difference.getChanged()) {
+                changed.setDirty(true);
+            }
+            removed.addAll(difference.getRemoved());
+        }
+    }
+
     @Override
     public Collection<ProtectedRegion> values() {
         return Collections.unmodifiableCollection(regions.values());

src/main/java/com/sk89q/worldguard/protection/managers/index/RegionIndex.java

@@ -160,6 +160,13 @@ public interface RegionIndex extends ChangeTracked {
      */
     RegionDifference getAndClearDifference();
 
+    /**
+     * Set the index to be dirty using the given difference.
+     *
+     * @param difference the difference
+     */
+    void setDirty(RegionDifference difference);
+
     /**
      * Get an unmodifiable collection of regions stored in this index.
      *

src/main/java/com/sk89q/worldguard/protection/managers/storage/driver/SQLDriver.java

@@ -75,7 +75,7 @@ public RegionStore get(String name) throws IOException {
         try {
             return new SQLRegionStore(config, getConnectionPool(), name);
         } catch (SQLException e) {
-            throw new IOException("Failed to get a connection pool for storing regions (are the SQL details correct?)");
+            throw new IOException("Failed to get a connection pool for storing regions (are the SQL details correct? is the SQL server online?)");
         }
     }
 

src/main/java/com/sk89q/worldguard/protection/managers/storage/sql/SQLRegionStore.java

@@ -48,6 +48,10 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -68,7 +72,6 @@ public class SQLRegionStore implements RegionStore {
     /**
      * Create a new instance.
      *
-     * @param name the name of this store
      * @param config a configuration object that configures a {@link Connection}
      * @param connectionPool a connection pool
      * @param worldName the name of the world to store regions by
@@ -227,7 +230,20 @@ private int chooseWorldId(String worldName) throws SQLException {
      * @throws SQLException thrown if the connection could not be created
      */
     private Connection getConnection() throws SQLException {
-        return connectionPool.getConnection();
+        Future<Connection> future = connectionPool.getAsyncConnection();
+        try {
+            return future.get(10, TimeUnit.SECONDS);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            future.cancel(true);
+            throw new SQLException("Failed to get connection -- interrupted");
+        } catch (ExecutionException e) {
+            future.cancel(true);
+            throw new SQLException("Failed to get connection; an error occurred", e);
+        } catch (TimeoutException e) {
+            future.cancel(true);
+            throw new SQLException("Failed to get connection; took too long to get a valid SQL connection (is the database server down?)");
+        }
     }
 
     /**