Don't unload blocks if save failed

Improve error handling in saveBlock()

diff --git a/src/database-dummy.cpp b/src/database-dummy.cpp
index c4794d281a282330f1e8ba2e55eb3199396e7678..7f715dd19bef81b0929fb2bacd248c44363cceb2 100644 (file)
--- a/src/database-dummy.cpp
+++ b/src/database-dummy.cpp
@@ -45,7 +45,7 @@ int Database_Dummy::Initialized(void)
 void Database_Dummy::beginSave() {}
 void Database_Dummy::endSave() {}
 
-void Database_Dummy::saveBlock(MapBlock *block)
+bool Database_Dummy::saveBlock(MapBlock *block)
 {
        DSTACK(__FUNCTION_NAME);
        /*
@@ -53,7 +53,10 @@ void Database_Dummy::saveBlock(MapBlock *block)
        */
        if(block->isDummy())
        {
-               return;
+               v3s16 p = block->getPos();
+               infostream<<"Database_Dummy::saveBlock(): WARNING: Not writing dummy block "
+                               <<"("<<p.X<<","<<p.Y<<","<<p.Z<<")"<<std::endl;
+               return true;
        }
 
        // Format used for writing
@@ -76,6 +79,7 @@ void Database_Dummy::saveBlock(MapBlock *block)
        m_database[getBlockAsInteger(p3d)] = tmp;
        // We just wrote it to the disk so clear modified flag
        block->resetModified();
+       return true;
 }
 
 MapBlock* Database_Dummy::loadBlock(v3s16 blockpos)

diff --git a/src/database-dummy.h b/src/database-dummy.h
index c0bee97c0c3d1077ace4c937549969d84da1a72b..d179d422603cba86cf97f08d601a8d2dbae9d47b 100644 (file)
--- a/src/database-dummy.h
+++ b/src/database-dummy.h
@@ -33,8 +33,8 @@ public:
        Database_Dummy(ServerMap *map);
        virtual void beginSave();
        virtual void endSave();
-        virtual void saveBlock(MapBlock *block);
-        virtual MapBlock* loadBlock(v3s16 blockpos);
+        virtual bool saveBlock(MapBlock *block);
+        virtual MapBlock *loadBlock(v3s16 blockpos);
         virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
         virtual int Initialized(void);
        ~Database_Dummy();

diff --git a/src/database-leveldb.cpp b/src/database-leveldb.cpp
index 9fe47b34ec9f9920dea439286df67dcacad58613..0cf685740fd70f9d69b9ca768fe26fc179590343 100644 (file)
--- a/src/database-leveldb.cpp
+++ b/src/database-leveldb.cpp
@@ -37,8 +37,8 @@ LevelDB databases
 #include "log.h"
 
 #define ENSURE_STATUS_OK(s) \
-       if (!s.ok()) { \
-               throw FileNotGoodException(std::string("LevelDB error: ") + s.ToString()); \
+       if (!(s).ok()) { \
+               throw FileNotGoodException(std::string("LevelDB error: ") + (s).ToString()); \
        }
 
 Database_LevelDB::Database_LevelDB(ServerMap *map, std::string savedir)
@@ -58,27 +58,29 @@ int Database_LevelDB::Initialized(void)
 void Database_LevelDB::beginSave() {}
 void Database_LevelDB::endSave() {}
 
-void Database_LevelDB::saveBlock(MapBlock *block)
+bool Database_LevelDB::saveBlock(MapBlock *block)
 {
        DSTACK(__FUNCTION_NAME);
+
+       v3s16 p3d = block->getPos();
+
        /*
                Dummy blocks are not written
        */
        if(block->isDummy())
        {
-               return;
+               errorstream << "WARNING: saveBlock: Not writing dummy block "
+                               << PP(p3d) << std::endl;
+               return true;
        }
 
        // Format used for writing
        u8 version = SER_FMT_VER_HIGHEST_WRITE;
-       // Get destination
-       v3s16 p3d = block->getPos();
 
        /*
                [0] u8 serialization version
                [1] data
        */
-
        std::ostringstream o(std::ios_base::binary);
        o.write((char*)&version, 1);
        // Write basic data
@@ -86,11 +88,17 @@ void Database_LevelDB::saveBlock(MapBlock *block)
        // Write block to database
        std::string tmp = o.str();
 
-       leveldb::Status status = m_database->Put(leveldb::WriteOptions(), i64tos(getBlockAsInteger(p3d)), tmp);
-       ENSURE_STATUS_OK(status);
+       leveldb::Status status = m_database->Put(leveldb::WriteOptions(),
+                       i64tos(getBlockAsInteger(p3d)), tmp);
+       if (!status.ok()) {
+               errorstream << "WARNING: saveBlock: LevelDB error saving block "
+                       << PP(p3d) << ": " << status.ToString() << std::endl;
+               return false;
+       }
 
        // We just wrote it to the disk so clear modified flag
        block->resetModified();
+       return true;
 }
 
 MapBlock* Database_LevelDB::loadBlock(v3s16 blockpos)

diff --git a/src/database-leveldb.h b/src/database-leveldb.h
index 5408f4ce6cdf124071d9a4d0e066667a1426c337..0c20aeaefb28d849543c5064f7eded2da50b23f5 100644 (file)
--- a/src/database-leveldb.h
+++ b/src/database-leveldb.h
@@ -36,8 +36,8 @@ public:
        Database_LevelDB(ServerMap *map, std::string savedir);
        virtual void beginSave();
        virtual void endSave();
-        virtual void saveBlock(MapBlock *block);
-        virtual MapBlock* loadBlock(v3s16 blockpos);
+        virtual bool saveBlock(MapBlock *block);
+        virtual MapBlock *loadBlock(v3s16 blockpos);
         virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
         virtual int Initialized(void);
        ~Database_LevelDB();

diff --git a/src/database-redis.cpp b/src/database-redis.cpp
index ff54753e6055bd62c5fc2535e6976bc23f1f3039..595fb20ecc18b7c9ac28fc942533fe2f25f53355 100644 (file)
--- a/src/database-redis.cpp
+++ b/src/database-redis.cpp
@@ -81,21 +81,24 @@ void Database_Redis::endSave() {
        freeReplyObject(reply);
 }
 
-void Database_Redis::saveBlock(MapBlock *block)
+bool Database_Redis::saveBlock(MapBlock *block)
 {
        DSTACK(__FUNCTION_NAME);
+
+       v3s16 p3d = block->getPos();
+
        /*
                Dummy blocks are not written
        */
        if(block->isDummy())
        {
-               return;
+               errorstream << "WARNING: saveBlock: Not writing dummy block "
+                               << PP(p3d) << std::endl;
+               return true;
        }
 
        // Format used for writing
        u8 version = SER_FMT_VER_HIGHEST_WRITE;
-       // Get destination
-       v3s16 p3d = block->getPos();
 
        /*
                [0] u8 serialization version
@@ -110,16 +113,26 @@ void Database_Redis::saveBlock(MapBlock *block)
        std::string tmp1 = o.str();
        std::string tmp2 = i64tos(getBlockAsInteger(p3d));
 
-       redisReply *reply;
-       reply = (redisReply*) redisCommand(ctx, "HSET %s %s %b", hash.c_str(), tmp2.c_str(), tmp1.c_str(), tmp1.size());
-       if(!reply)
-               throw FileNotGoodException(std::string("redis command 'HSET %s %s %b' failed: ") + ctx->errstr);
-       if(reply->type == REDIS_REPLY_ERROR)
-               throw FileNotGoodException("Failed to store block in Database");
-       freeReplyObject(reply);
+       redisReply *reply = (redisReply *)redisCommand(ctx, "HSET %s %s %b",
+                       hash.c_str(), tmp2.c_str(), tmp1.c_str(), tmp1.size());
+       if (!reply) {
+               errorstream << "WARNING: saveBlock: redis command 'HSET' failed on "
+                       "block " << PP(p3d) << ": " << ctx->errstr << std::endl;
+               freeReplyObject(reply);
+               return false;
+       }
+
+       if (reply->type == REDIS_REPLY_ERROR) {
+               errorstream << "WARNING: saveBlock: save block " << PP(p3d)
+                       << "failed" << std::endl;
+               freeReplyObject(reply);
+               return false;
+       }
 
        // We just wrote it to the disk so clear modified flag
        block->resetModified();
+       freeReplyObject(reply);
+       return true;
 }
 
 MapBlock* Database_Redis::loadBlock(v3s16 blockpos)

diff --git a/src/database-redis.h b/src/database-redis.h
index da76775d409c4530b7c49f081f1c825936a7ac87..92983ee7bfb2ed0e891d085ce88a92ca497c4777 100644 (file)
--- a/src/database-redis.h
+++ b/src/database-redis.h
@@ -36,8 +36,8 @@ public:
        Database_Redis(ServerMap *map, std::string savedir);
        virtual void beginSave();
        virtual void endSave();
-       virtual void saveBlock(MapBlock *block);
-       virtual MapBlock* loadBlock(v3s16 blockpos);
+       virtual bool saveBlock(MapBlock *block);
+       virtual MapBlock *loadBlock(v3s16 blockpos);
        virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
        virtual int Initialized(void);
        ~Database_Redis();

diff --git a/src/database-sqlite3.cpp b/src/database-sqlite3.cpp
index 3ec13a1a26acdcc3ee5e0613ae7bbed8ce8891a2..fb76d8eae6d672d909c8e61e1b8c7ee4921a2ee7 100644 (file)
--- a/src/database-sqlite3.cpp
+++ b/src/database-sqlite3.cpp
@@ -136,25 +136,24 @@ void Database_SQLite3::verifyDatabase() {
        infostream<<"ServerMap: SQLite3 database opened"<<std::endl;
 }
 
-void Database_SQLite3::saveBlock(MapBlock *block)
+bool Database_SQLite3::saveBlock(MapBlock *block)
 {
        DSTACK(__FUNCTION_NAME);
+
+       v3s16 p3d = block->getPos();
+
        /*
-               Dummy blocks are not written
+               Dummy blocks are not written, but is not a save failure
        */
        if(block->isDummy())
        {
-               /*v3s16 p = block->getPos();
-               infostream<<"Database_SQLite3::saveBlock(): WARNING: Not writing dummy block "
-                               <<"("<<p.X<<","<<p.Y<<","<<p.Z<<")"<<std::endl;*/
-               return;
+               errorstream << "WARNING: saveBlock: Not writing dummy block "
+                               << PP(p3d) << std::endl;
+               return true;
        }
 
        // Format used for writing
        u8 version = SER_FMT_VER_HIGHEST_WRITE;
-       // Get destination
-       v3s16 p3d = block->getPos();
-
 
 #if 0
        v2s16 p2d(p3d.X, p3d.Z);
@@ -176,29 +175,39 @@ void Database_SQLite3::saveBlock(MapBlock *block)
 
        std::ostringstream o(std::ios_base::binary);
 
-       o.write((char*)&version, 1);
+       o.write((char *)&version, 1);
 
        // Write basic data
        block->serialize(o, version, true);
 
        // Write block to database
-
        std::string tmp = o.str();
-       const char *bytes = tmp.c_str();
-
-       if(sqlite3_bind_int64(m_database_write, 1, getBlockAsInteger(p3d)) != SQLITE_OK)
-               errorstream<<"WARNING: Block position failed to bind: "<<sqlite3_errmsg(m_database)<<std::endl;
-       if(sqlite3_bind_blob(m_database_write, 2, (void *)bytes, o.tellp(), NULL) != SQLITE_OK) // TODO this mught not be the right length
-               errorstream<<"WARNING: Block data failed to bind: "<<sqlite3_errmsg(m_database)<<std::endl;
-       int written = sqlite3_step(m_database_write);
-       if(written != SQLITE_DONE)
-               errorstream<<"WARNING: Block failed to save ("<<p3d.X<<", "<<p3d.Y<<", "<<p3d.Z<<") "
-               <<sqlite3_errmsg(m_database)<<std::endl;
-       // Make ready for later reuse
-       sqlite3_reset(m_database_write);
+
+       if (sqlite3_bind_int64(m_database_write, 1, getBlockAsInteger(p3d)) != SQLITE_OK) {
+               errorstream << "WARNING: saveBlock: Block position failed to bind: "
+                       << PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
+               sqlite3_reset(m_database_write);
+               return false;
+       }
+
+       if (sqlite3_bind_blob(m_database_write, 2, (void *)tmp.c_str(), tmp.size(), NULL) != SQLITE_OK) {
+               errorstream << "WARNING: saveBlock: Block data failed to bind: "
+                       << PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
+               sqlite3_reset(m_database_write);
+               return false;
+       }
+
+       if (sqlite3_step(m_database_write) != SQLITE_DONE) {
+               errorstream << "WARNING: saveBlock: Block failed to save "
+                       << PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
+               sqlite3_reset(m_database_write);
+               return false;
+       }
 
        // We just wrote it to the disk so clear modified flag
        block->resetModified();
+       sqlite3_reset(m_database_write);
+       return true;
 }
 
 MapBlock* Database_SQLite3::loadBlock(v3s16 blockpos)

diff --git a/src/database-sqlite3.h b/src/database-sqlite3.h
index f426f46189287c7a1422e0e1368dcde0a0e71dd5..1753072e863013715d6ebafc5fbeb8bbe323ceff 100644 (file)
--- a/src/database-sqlite3.h
+++ b/src/database-sqlite3.h
@@ -36,8 +36,8 @@ public:
         virtual void beginSave();
         virtual void endSave();
 
-        virtual void saveBlock(MapBlock *block);
-        virtual MapBlock* loadBlock(v3s16 blockpos);
+        virtual bool saveBlock(MapBlock *block);
+        virtual MapBlock *loadBlock(v3s16 blockpos);
         virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
         virtual int Initialized(void);
        ~Database_SQLite3();

diff --git a/src/database.h b/src/database.h
index f009877d244cac6a780bf44f779f5359905b1772..d8669dd9b8a3834fc0f6206815968ace644ad1d9 100644 (file)
--- a/src/database.h
+++ b/src/database.h
@@ -24,19 +24,23 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "irr_v3d.h"
 #include "irrlichttypes.h"
 
+#ifndef PP
+       #define PP(x) "("<<(x).X<<","<<(x).Y<<","<<(x).Z<<")"
+#endif
+
 class MapBlock;
 
 class Database
 {
 public:
-       virtual void beginSave()=0;
-       virtual void endSave()=0;
+       virtual void beginSave() = 0;
+       virtual void endSave() = 0;
 
-       virtual void saveBlock(MapBlock *block)=0;
-       virtual MapBlock* loadBlock(v3s16 blockpos)=0;
+       virtual bool saveBlock(MapBlock *block) = 0;
+       virtual MapBlock *loadBlock(v3s16 blockpos) = 0;
        s64 getBlockAsInteger(const v3s16 pos) const;
        v3s16 getIntegerAsBlock(s64 i) const;
-       virtual void listAllLoadableBlocks(std::list<v3s16> &dst)=0;
+       virtual void listAllLoadableBlocks(std::list<v3s16> &dst) = 0;
        virtual int Initialized(void)=0;
        virtual ~Database() {};
 };

diff --git a/src/map.cpp b/src/map.cpp
index 35bd485a490eee1670652556baa2632a31956721..9c06750b81cd38e665bc8745fada7ea10c3318e1 100644 (file)
--- a/src/map.cpp
+++ b/src/map.cpp
@@ -1473,11 +1473,11 @@ void Map::timerUpdate(float dtime, float unload_timeout,
                                v3s16 p = block->getPos();
 
                                // Save if modified
-                               if(block->getModified() != MOD_STATE_CLEAN
-                                               && save_before_unloading)
+                               if (block->getModified() != MOD_STATE_CLEAN && save_before_unloading)
                                {
                                        modprofiler.add(block->getModifiedReason(), 1);
-                                       saveBlock(block);
+                                       if (!saveBlock(block))
+                                               continue;
                                        saved_blocks_count++;
                                }
 
@@ -3253,20 +3253,23 @@ bool ServerMap::loadSectorFull(v2s16 p2d)
 }
 #endif
 
-void ServerMap::beginSave() {
+void ServerMap::beginSave()
+{
        dbase->beginSave();
 }
 
-void ServerMap::endSave() {
+void ServerMap::endSave()
+{
        dbase->endSave();
 }
 
-void ServerMap::saveBlock(MapBlock *block)
+bool ServerMap::saveBlock(MapBlock *block)
 {
-  dbase->saveBlock(block);
+       return dbase->saveBlock(block);
 }
 
-void ServerMap::loadBlock(std::string sectordir, std::string blockfile, MapSector *sector, bool save_after_load)
+void ServerMap::loadBlock(std::string sectordir, std::string blockfile,
+               MapSector *sector, bool save_after_load)
 {
        DSTACK(__FUNCTION_NAME);
 

diff --git a/src/map.h b/src/map.h
index 7f482929e179b29222349b36eddc55e070b01405..4972046f535590329eaae6100ea18aea586122ae 100644 (file)
--- a/src/map.h
+++ b/src/map.h
@@ -270,7 +270,7 @@ public:
 
        // Server implements this.
        // Client leaves it as no-op.
-       virtual void saveBlock(MapBlock *block){};
+       virtual bool saveBlock(MapBlock *block){ return false; };
 
        /*
                Updates usage timers and unloads unused blocks and sectors.
@@ -485,7 +485,7 @@ public:
        // Returns true if sector now resides in memory
        //bool deFlushSector(v2s16 p2d);
 
-       void saveBlock(MapBlock *block);
+       bool saveBlock(MapBlock *block);
        // This will generate a sector with getSector if not found.
        void loadBlock(std::string sectordir, std::string blockfile, MapSector *sector, bool save_after_load=false);
        MapBlock* loadBlock(v3s16 p);