Fix various player save issues (performance penalty on sql backends + bugs)

nerzhul · nerzhul · commit c1d7dbfc38e8 · 2019-01-04T10:20:04.000+01:00
* PostgreSQL &amp; SQLite3 doesn't setModified(false) on RemotePlayer, then player is saved on each server save call. This results in heavy useless writes.
* PostgreSQL &amp; SQLite3 ack engine meta write whereas db commit hasn't been performed. If commit failed write has failed. We mustn't notify engine write is done.
* serializing player meta must not setModified(false) because it didn't ensure write has been done
* add RemotePlayer::on_successfull_save callback to do the flag update on a successful save
diff --git a/src/database/database-files.cpp b/src/database/database-files.cpp
@@ -105,7 +105,8 @@ void PlayerDatabaseFiles::savePlayer(RemotePlayer *player)
 	if (!fs::safeWriteToFile(path, ss.str())) {
 		infostream << "Failed to write " << path << std::endl;
 	}
-	player->setModified(false);
+
+	player->on_successful_save();
 }
 
 bool PlayerDatabaseFiles::removePlayer(const std::string &name)
diff --git a/src/database/database-postgresql.cpp b/src/database/database-postgresql.cpp
@@ -527,8 +527,9 @@ void PlayerDatabasePostgreSQL::savePlayer(RemotePlayer *player)
 		};
 		execPrepared("save_player_metadata", 3, meta_values);
 	}
-	sao->getMeta().setModified(false);
 	endSave();
+
+	player->on_successful_save();
 }
 
 bool PlayerDatabasePostgreSQL::loadPlayer(RemotePlayer *player, PlayerSAO *sao)
diff --git a/src/database/database-sqlite3.cpp b/src/database/database-sqlite3.cpp
@@ -528,9 +528,10 @@ void PlayerDatabaseSQLite3::savePlayer(RemotePlayer *player)
 		sqlite3_vrfy(sqlite3_step(m_stmt_player_metadata_add), SQLITE_DONE);
 		sqlite3_reset(m_stmt_player_metadata_add);
 	}
-	sao->getMeta().setModified(false);
 
 	endSave();
+
+	player->on_successful_save();
 }
 
 bool PlayerDatabaseSQLite3::loadPlayer(RemotePlayer *player, PlayerSAO *sao)
diff --git a/src/remoteplayer.cpp b/src/remoteplayer.cpp
@@ -79,8 +79,6 @@ void RemotePlayer::serializeExtraAttributes(std::string &output)
 	}
 
 	output = fastWriteJson(json_root);
-
-	m_sao->getMeta().setModified(false);
 }
 
 
@@ -225,3 +223,10 @@ const RemotePlayerChatResult RemotePlayer::canSendChatMessage()
 	m_chat_message_allowance -= 1.0f;
 	return RPLAYER_CHATRESULT_OK;
 }
+
+void RemotePlayer::on_successful_save()
+{
+	setModified(false);
+	if (m_sao)
+		m_sao->getMeta().setModified(false);
+}
diff --git a/src/remoteplayer.h b/src/remoteplayer.h
@@ -139,6 +139,8 @@ class RemotePlayer : public Player
 
 	void setPeerId(session_t peer_id) { m_peer_id = peer_id; }
 
+	void on_successful_save();
+
 private:
 	/*
 		serialize() writes a bunch of text that can contain

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,8 @@ void PlayerDatabaseFiles::savePlayer(RemotePlayer *player)`
`105`	`105`	`if (!fs::safeWriteToFile(path, ss.str())) {`
`106`	`106`	`infostream << "Failed to write " << path << std::endl;`
`107`	`107`	`}`
`108`		`- player->setModified(false);`
	`108`	`+`
	`109`	`+ player->on_successful_save();`
`109`	`110`	`}`
`110`	`111`
`111`	`112`	`bool PlayerDatabaseFiles::removePlayer(const std::string &name)`
Original file line number	Diff line number	Diff line change
`@@ -527,8 +527,9 @@ void PlayerDatabasePostgreSQL::savePlayer(RemotePlayer *player)`
`527`	`527`	`};`
`528`	`528`	`execPrepared("save_player_metadata", 3, meta_values);`
`529`	`529`	`}`
`530`		`- sao->getMeta().setModified(false);`
`531`	`530`	`endSave();`
	`531`	`+`
	`532`	`+ player->on_successful_save();`
`532`	`533`	`}`
`533`	`534`
`534`	`535`	`bool PlayerDatabasePostgreSQL::loadPlayer(RemotePlayer player, PlayerSAO sao)`
Original file line number	Diff line number	Diff line change
`@@ -528,9 +528,10 @@ void PlayerDatabaseSQLite3::savePlayer(RemotePlayer *player)`
`528`	`528`	`sqlite3_vrfy(sqlite3_step(m_stmt_player_metadata_add), SQLITE_DONE);`
`529`	`529`	`sqlite3_reset(m_stmt_player_metadata_add);`
`530`	`530`	`}`
`531`		`- sao->getMeta().setModified(false);`
`532`	`531`
`533`	`532`	`endSave();`
	`533`	`+`
	`534`	`+ player->on_successful_save();`
`534`	`535`	`}`
`535`	`536`
`536`	`537`	`bool PlayerDatabaseSQLite3::loadPlayer(RemotePlayer player, PlayerSAO sao)`
Original file line number	Diff line number	Diff line change
`@@ -79,8 +79,6 @@ void RemotePlayer::serializeExtraAttributes(std::string &output)`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`output = fastWriteJson(json_root);`
`82`		`-`
`83`		`- m_sao->getMeta().setModified(false);`
`84`	`82`	`}`
`85`	`83`
`86`	`84`
`@@ -225,3 +223,10 @@ const RemotePlayerChatResult RemotePlayer::canSendChatMessage()`
`225`	`223`	`m_chat_message_allowance -= 1.0f;`
`226`	`224`	`return RPLAYER_CHATRESULT_OK;`
`227`	`225`	`}`
	`226`	`+`
	`227`	`+void RemotePlayer::on_successful_save()`
	`228`	`+{`
	`229`	`+ setModified(false);`
	`230`	`+ if (m_sao)`
	`231`	`+ m_sao->getMeta().setModified(false);`
	`232`	`+}`