Skip to content

Commit 8b3ed78

Browse files
kwolekrkwolekr
committedJul 7, 2014
Don't unload blocks if save failed
Improve error handling in saveBlock()
·
5.11.00.4.11
1 parent e14c4cd commit 8b3ed78

11 files changed

+109
-68
lines changed
 

Diff for: ‎src/database-dummy.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,18 @@ int Database_Dummy::Initialized(void)
4545
void Database_Dummy::beginSave() {}
4646
void Database_Dummy::endSave() {}
4747

48-
void Database_Dummy::saveBlock(MapBlock *block)
48+
bool Database_Dummy::saveBlock(MapBlock *block)
4949
{
5050
DSTACK(__FUNCTION_NAME);
5151
/*
5252
Dummy blocks are not written
5353
*/
5454
if(block->isDummy())
5555
{
56-
return;
56+
v3s16 p = block->getPos();
57+
infostream<<"Database_Dummy::saveBlock(): WARNING: Not writing dummy block "
58+
<<"("<<p.X<<","<<p.Y<<","<<p.Z<<")"<<std::endl;
59+
return true;
5760
}
5861

5962
// Format used for writing
@@ -76,6 +79,7 @@ void Database_Dummy::saveBlock(MapBlock *block)
7679
m_database[getBlockAsInteger(p3d)] = tmp;
7780
// We just wrote it to the disk so clear modified flag
7881
block->resetModified();
82+
return true;
7983
}
8084

8185
MapBlock* Database_Dummy::loadBlock(v3s16 blockpos)

Diff for: ‎src/database-dummy.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ class Database_Dummy : public Database
3333
Database_Dummy(ServerMap *map);
3434
virtual void beginSave();
3535
virtual void endSave();
36-
virtual void saveBlock(MapBlock *block);
37-
virtual MapBlock* loadBlock(v3s16 blockpos);
36+
virtual bool saveBlock(MapBlock *block);
37+
virtual MapBlock *loadBlock(v3s16 blockpos);
3838
virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
3939
virtual int Initialized(void);
4040
~Database_Dummy();

Diff for: ‎src/database-leveldb.cpp

+17-9
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ LevelDB databases
3737
#include "log.h"
3838

3939
#define ENSURE_STATUS_OK(s) \
40-
if (!s.ok()) { \
41-
throw FileNotGoodException(std::string("LevelDB error: ") + s.ToString()); \
40+
if (!(s).ok()) { \
41+
throw FileNotGoodException(std::string("LevelDB error: ") + (s).ToString()); \
4242
}
4343

4444
Database_LevelDB::Database_LevelDB(ServerMap *map, std::string savedir)
@@ -58,39 +58,47 @@ int Database_LevelDB::Initialized(void)
5858
void Database_LevelDB::beginSave() {}
5959
void Database_LevelDB::endSave() {}
6060

61-
void Database_LevelDB::saveBlock(MapBlock *block)
61+
bool Database_LevelDB::saveBlock(MapBlock *block)
6262
{
6363
DSTACK(__FUNCTION_NAME);
64+
65+
v3s16 p3d = block->getPos();
66+
6467
/*
6568
Dummy blocks are not written
6669
*/
6770
if(block->isDummy())
6871
{
69-
return;
72+
errorstream << "WARNING: saveBlock: Not writing dummy block "
73+
<< PP(p3d) << std::endl;
74+
return true;
7075
}
7176

7277
// Format used for writing
7378
u8 version = SER_FMT_VER_HIGHEST_WRITE;
74-
// Get destination
75-
v3s16 p3d = block->getPos();
7679

7780
/*
7881
[0] u8 serialization version
7982
[1] data
8083
*/
81-
8284
std::ostringstream o(std::ios_base::binary);
8385
o.write((char*)&version, 1);
8486
// Write basic data
8587
block->serialize(o, version, true);
8688
// Write block to database
8789
std::string tmp = o.str();
8890

89-
leveldb::Status status = m_database->Put(leveldb::WriteOptions(), i64tos(getBlockAsInteger(p3d)), tmp);
90-
ENSURE_STATUS_OK(status);
91+
leveldb::Status status = m_database->Put(leveldb::WriteOptions(),
92+
i64tos(getBlockAsInteger(p3d)), tmp);
93+
if (!status.ok()) {
94+
errorstream << "WARNING: saveBlock: LevelDB error saving block "
95+
<< PP(p3d) << ": " << status.ToString() << std::endl;
96+
return false;
97+
}
9198

9299
// We just wrote it to the disk so clear modified flag
93100
block->resetModified();
101+
return true;
94102
}
95103

96104
MapBlock* Database_LevelDB::loadBlock(v3s16 blockpos)

Diff for: ‎src/database-leveldb.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ class Database_LevelDB : public Database
3636
Database_LevelDB(ServerMap *map, std::string savedir);
3737
virtual void beginSave();
3838
virtual void endSave();
39-
virtual void saveBlock(MapBlock *block);
40-
virtual MapBlock* loadBlock(v3s16 blockpos);
39+
virtual bool saveBlock(MapBlock *block);
40+
virtual MapBlock *loadBlock(v3s16 blockpos);
4141
virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
4242
virtual int Initialized(void);
4343
~Database_LevelDB();

Diff for: ‎src/database-redis.cpp

+24-11
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,24 @@ void Database_Redis::endSave() {
8181
freeReplyObject(reply);
8282
}
8383

84-
void Database_Redis::saveBlock(MapBlock *block)
84+
bool Database_Redis::saveBlock(MapBlock *block)
8585
{
8686
DSTACK(__FUNCTION_NAME);
87+
88+
v3s16 p3d = block->getPos();
89+
8790
/*
8891
Dummy blocks are not written
8992
*/
9093
if(block->isDummy())
9194
{
92-
return;
95+
errorstream << "WARNING: saveBlock: Not writing dummy block "
96+
<< PP(p3d) << std::endl;
97+
return true;
9398
}
9499

95100
// Format used for writing
96101
u8 version = SER_FMT_VER_HIGHEST_WRITE;
97-
// Get destination
98-
v3s16 p3d = block->getPos();
99102

100103
/*
101104
[0] u8 serialization version
@@ -110,16 +113,26 @@ void Database_Redis::saveBlock(MapBlock *block)
110113
std::string tmp1 = o.str();
111114
std::string tmp2 = i64tos(getBlockAsInteger(p3d));
112115

113-
redisReply *reply;
114-
reply = (redisReply*) redisCommand(ctx, "HSET %s %s %b", hash.c_str(), tmp2.c_str(), tmp1.c_str(), tmp1.size());
115-
if(!reply)
116-
throw FileNotGoodException(std::string("redis command 'HSET %s %s %b' failed: ") + ctx->errstr);
117-
if(reply->type == REDIS_REPLY_ERROR)
118-
throw FileNotGoodException("Failed to store block in Database");
119-
freeReplyObject(reply);
116+
redisReply *reply = (redisReply *)redisCommand(ctx, "HSET %s %s %b",
117+
hash.c_str(), tmp2.c_str(), tmp1.c_str(), tmp1.size());
118+
if (!reply) {
119+
errorstream << "WARNING: saveBlock: redis command 'HSET' failed on "
120+
"block " << PP(p3d) << ": " << ctx->errstr << std::endl;
121+
freeReplyObject(reply);
122+
return false;
123+
}
124+
125+
if (reply->type == REDIS_REPLY_ERROR) {
126+
errorstream << "WARNING: saveBlock: save block " << PP(p3d)
127+
<< "failed" << std::endl;
128+
freeReplyObject(reply);
129+
return false;
130+
}
120131

121132
// We just wrote it to the disk so clear modified flag
122133
block->resetModified();
134+
freeReplyObject(reply);
135+
return true;
123136
}
124137

125138
MapBlock* Database_Redis::loadBlock(v3s16 blockpos)

Diff for: ‎src/database-redis.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ class Database_Redis : public Database
3636
Database_Redis(ServerMap *map, std::string savedir);
3737
virtual void beginSave();
3838
virtual void endSave();
39-
virtual void saveBlock(MapBlock *block);
40-
virtual MapBlock* loadBlock(v3s16 blockpos);
39+
virtual bool saveBlock(MapBlock *block);
40+
virtual MapBlock *loadBlock(v3s16 blockpos);
4141
virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
4242
virtual int Initialized(void);
4343
~Database_Redis();

Diff for: ‎src/database-sqlite3.cpp

+32-23
Original file line numberDiff line numberDiff line change
@@ -136,25 +136,24 @@ void Database_SQLite3::verifyDatabase() {
136136
infostream<<"ServerMap: SQLite3 database opened"<<std::endl;
137137
}
138138

139-
void Database_SQLite3::saveBlock(MapBlock *block)
139+
bool Database_SQLite3::saveBlock(MapBlock *block)
140140
{
141141
DSTACK(__FUNCTION_NAME);
142+
143+
v3s16 p3d = block->getPos();
144+
142145
/*
143-
Dummy blocks are not written
146+
Dummy blocks are not written, but is not a save failure
144147
*/
145148
if(block->isDummy())
146149
{
147-
/*v3s16 p = block->getPos();
148-
infostream<<"Database_SQLite3::saveBlock(): WARNING: Not writing dummy block "
149-
<<"("<<p.X<<","<<p.Y<<","<<p.Z<<")"<<std::endl;*/
150-
return;
150+
errorstream << "WARNING: saveBlock: Not writing dummy block "
151+
<< PP(p3d) << std::endl;
152+
return true;
151153
}
152154

153155
// Format used for writing
154156
u8 version = SER_FMT_VER_HIGHEST_WRITE;
155-
// Get destination
156-
v3s16 p3d = block->getPos();
157-
158157

159158
#if 0
160159
v2s16 p2d(p3d.X, p3d.Z);
@@ -176,29 +175,39 @@ void Database_SQLite3::saveBlock(MapBlock *block)
176175

177176
std::ostringstream o(std::ios_base::binary);
178177

179-
o.write((char*)&version, 1);
178+
o.write((char *)&version, 1);
180179

181180
// Write basic data
182181
block->serialize(o, version, true);
183182

184183
// Write block to database
185-
186184
std::string tmp = o.str();
187-
const char *bytes = tmp.c_str();
188-
189-
if(sqlite3_bind_int64(m_database_write, 1, getBlockAsInteger(p3d)) != SQLITE_OK)
190-
errorstream<<"WARNING: Block position failed to bind: "<<sqlite3_errmsg(m_database)<<std::endl;
191-
if(sqlite3_bind_blob(m_database_write, 2, (void *)bytes, o.tellp(), NULL) != SQLITE_OK) // TODO this mught not be the right length
192-
errorstream<<"WARNING: Block data failed to bind: "<<sqlite3_errmsg(m_database)<<std::endl;
193-
int written = sqlite3_step(m_database_write);
194-
if(written != SQLITE_DONE)
195-
errorstream<<"WARNING: Block failed to save ("<<p3d.X<<", "<<p3d.Y<<", "<<p3d.Z<<") "
196-
<<sqlite3_errmsg(m_database)<<std::endl;
197-
// Make ready for later reuse
198-
sqlite3_reset(m_database_write);
185+
186+
if (sqlite3_bind_int64(m_database_write, 1, getBlockAsInteger(p3d)) != SQLITE_OK) {
187+
errorstream << "WARNING: saveBlock: Block position failed to bind: "
188+
<< PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
189+
sqlite3_reset(m_database_write);
190+
return false;
191+
}
192+
193+
if (sqlite3_bind_blob(m_database_write, 2, (void *)tmp.c_str(), tmp.size(), NULL) != SQLITE_OK) {
194+
errorstream << "WARNING: saveBlock: Block data failed to bind: "
195+
<< PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
196+
sqlite3_reset(m_database_write);
197+
return false;
198+
}
199+
200+
if (sqlite3_step(m_database_write) != SQLITE_DONE) {
201+
errorstream << "WARNING: saveBlock: Block failed to save "
202+
<< PP(p3d) << ": " << sqlite3_errmsg(m_database) << std::endl;
203+
sqlite3_reset(m_database_write);
204+
return false;
205+
}
199206

200207
// We just wrote it to the disk so clear modified flag
201208
block->resetModified();
209+
sqlite3_reset(m_database_write);
210+
return true;
202211
}
203212

204213
MapBlock* Database_SQLite3::loadBlock(v3s16 blockpos)

Diff for: ‎src/database-sqlite3.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ class Database_SQLite3 : public Database
3636
virtual void beginSave();
3737
virtual void endSave();
3838

39-
virtual void saveBlock(MapBlock *block);
40-
virtual MapBlock* loadBlock(v3s16 blockpos);
39+
virtual bool saveBlock(MapBlock *block);
40+
virtual MapBlock *loadBlock(v3s16 blockpos);
4141
virtual void listAllLoadableBlocks(std::list<v3s16> &dst);
4242
virtual int Initialized(void);
4343
~Database_SQLite3();

Diff for: ‎src/database.h

+9-5
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,23 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2424
#include "irr_v3d.h"
2525
#include "irrlichttypes.h"
2626

27+
#ifndef PP
28+
#define PP(x) "("<<(x).X<<","<<(x).Y<<","<<(x).Z<<")"
29+
#endif
30+
2731
class MapBlock;
2832

2933
class Database
3034
{
3135
public:
32-
virtual void beginSave()=0;
33-
virtual void endSave()=0;
36+
virtual void beginSave() = 0;
37+
virtual void endSave() = 0;
3438

35-
virtual void saveBlock(MapBlock *block)=0;
36-
virtual MapBlock* loadBlock(v3s16 blockpos)=0;
39+
virtual bool saveBlock(MapBlock *block) = 0;
40+
virtual MapBlock *loadBlock(v3s16 blockpos) = 0;
3741
s64 getBlockAsInteger(const v3s16 pos) const;
3842
v3s16 getIntegerAsBlock(s64 i) const;
39-
virtual void listAllLoadableBlocks(std::list<v3s16> &dst)=0;
43+
virtual void listAllLoadableBlocks(std::list<v3s16> &dst) = 0;
4044
virtual int Initialized(void)=0;
4145
virtual ~Database() {};
4246
};

Diff for: ‎src/map.cpp

+11-8
Original file line numberDiff line numberDiff line change
@@ -1473,11 +1473,11 @@ void Map::timerUpdate(float dtime, float unload_timeout,
14731473
v3s16 p = block->getPos();
14741474

14751475
// Save if modified
1476-
if(block->getModified() != MOD_STATE_CLEAN
1477-
&& save_before_unloading)
1476+
if (block->getModified() != MOD_STATE_CLEAN && save_before_unloading)
14781477
{
14791478
modprofiler.add(block->getModifiedReason(), 1);
1480-
saveBlock(block);
1479+
if (!saveBlock(block))
1480+
continue;
14811481
saved_blocks_count++;
14821482
}
14831483

@@ -3253,20 +3253,23 @@ bool ServerMap::loadSectorFull(v2s16 p2d)
32533253
}
32543254
#endif
32553255

3256-
void ServerMap::beginSave() {
3256+
void ServerMap::beginSave()
3257+
{
32573258
dbase->beginSave();
32583259
}
32593260

3260-
void ServerMap::endSave() {
3261+
void ServerMap::endSave()
3262+
{
32613263
dbase->endSave();
32623264
}
32633265

3264-
void ServerMap::saveBlock(MapBlock *block)
3266+
bool ServerMap::saveBlock(MapBlock *block)
32653267
{
3266-
dbase->saveBlock(block);
3268+
return dbase->saveBlock(block);
32673269
}
32683270

3269-
void ServerMap::loadBlock(std::string sectordir, std::string blockfile, MapSector *sector, bool save_after_load)
3271+
void ServerMap::loadBlock(std::string sectordir, std::string blockfile,
3272+
MapSector *sector, bool save_after_load)
32703273
{
32713274
DSTACK(__FUNCTION_NAME);
32723275

Diff for: ‎src/map.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ class Map /*: public NodeContainer*/
270270

271271
// Server implements this.
272272
// Client leaves it as no-op.
273-
virtual void saveBlock(MapBlock *block){};
273+
virtual bool saveBlock(MapBlock *block){ return false; };
274274

275275
/*
276276
Updates usage timers and unloads unused blocks and sectors.
@@ -485,7 +485,7 @@ class ServerMap : public Map
485485
// Returns true if sector now resides in memory
486486
//bool deFlushSector(v2s16 p2d);
487487

488-
void saveBlock(MapBlock *block);
488+
bool saveBlock(MapBlock *block);
489489
// This will generate a sector with getSector if not found.
490490
void loadBlock(std::string sectordir, std::string blockfile, MapSector *sector, bool save_after_load=false);
491491
MapBlock* loadBlock(v3s16 p);

0 commit comments

Comments
 (0)
Please sign in to comment.