From dd9d5ae6bd23535ee0a57e7a0dfb4adf2af1952f Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 07:16:48 +0000 Subject: [PATCH 1/3] more tracking of creation --- .../serverDatabase/src/shared/Persister.cpp | 23 ------------------- .../serverGame/src/shared/core/GameServer.cpp | 6 +++++ 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index a28fb55e..e9d62df0 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -214,8 +214,6 @@ void Persister::update(real updateTime) void Persister::onFrameBarrierReached() { - pad.lock(); - if (m_newCharacterTaskQueue->getNumPendingTasks() == 0) { ServerSnapshotMap delayedSaves; @@ -281,8 +279,6 @@ void Persister::onFrameBarrierReached() taskQueue->report(); } } - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -392,24 +388,16 @@ void Persister::startSave(void) */ Snapshot * Persister::getSnapshotForObject(const NetworkId &networkId, uint32 serverId) { - pad.lock(); - auto i = m_objectSnapshotMap.find(networkId); if (i!=m_objectSnapshotMap.end()) { NOT_NULL(i->second); - - pad.unlock(); - return i->second; } else { Snapshot *snap = getSnapshotForServer(serverId); m_objectSnapshotMap[networkId]=snap; - - pad.unlock(); - return snap; } } @@ -426,8 +414,6 @@ bool Persister::hasDataForObject(const NetworkId &objectId) const Snapshot *Persister::getSnapshotForServer(uint32 serverId) { - pad.lock(); - if (serverId==0) { if (!m_arbitraryGameDataSnapshot) @@ -435,9 +421,6 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) m_arbitraryGameDataSnapshot = makeSnapshot(DB::ModeQuery::mode_UPDATE); m_currentSnapshots[0] = m_arbitraryGameDataSnapshot; } - - pad.unlock(); - return m_arbitraryGameDataSnapshot; } else @@ -452,18 +435,12 @@ Snapshot *Persister::getSnapshotForServer(uint32 serverId) if (!m_arbitraryGameDataSnapshot) { m_arbitraryGameDataSnapshot = snap; } - - pad.unlock(); - return snap; } else { NOT_NULL (j->second); - - pad.unlock(); - return j->second; } } diff --git a/engine/server/library/serverGame/src/shared/core/GameServer.cpp b/engine/server/library/serverGame/src/shared/core/GameServer.cpp index e8f25f32..e760ae70 100755 --- a/engine/server/library/serverGame/src/shared/core/GameServer.cpp +++ b/engine/server/library/serverGame/src/shared/core/GameServer.cpp @@ -4769,10 +4769,14 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse // ---------------------------------------------------------------------- // Set up the PlayerObject + WARNING(true, ("Setting up the PlayerObject for %s", vrn.getName().c_str())); + ServerObject *playerServerObject = ServerWorld::createNewObject(ConfigServerGame::getPlayerObjectTemplate(), *newCharacterObject, false); PlayerObject *play = dynamic_cast(playerServerObject); if (play) { + WARNING(true, ("Player object is good...")); + play->setStationId(createMessage->getStationId()); play->setBornDate(); play->setSkillTemplate(createMessage->getSkillTemplate(), true); @@ -4807,6 +4811,8 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse WARNING_STRICT_FATAL(true,("Unable to create PlayerObject for new character %s.\n",newCharacterObject->getNetworkId().getValueString().c_str())); } + + WARNING(true, ("Setting scene and sending to DB for persist.")); newCharacterObject->setSceneIdOnThisAndContents(createMessage->getPlanetName()); // ---------------------------------------------------------------------- From 9e8beefada276da0cf69d1856e78664d2d277dd3 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 08:45:40 +0000 Subject: [PATCH 2/3] general cleanup; remove those mutexes and debug output as they didn't get us anywhere --- .../serverDatabase/src/shared/Persister.cpp | 92 +++++++------------ .../serverDatabase/src/shared/Persister.h | 4 - .../serverGame/src/shared/core/GameServer.cpp | 7 -- 3 files changed, 35 insertions(+), 68 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Persister.cpp b/engine/server/library/serverDatabase/src/shared/Persister.cpp index e9d62df0..c6f12b2e 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.cpp +++ b/engine/server/library/serverDatabase/src/shared/Persister.cpp @@ -303,8 +303,6 @@ void Persister::startSave(void) m_newObjectCount=0; - pad.lock(); - // delete any characters for this save cycle if (m_charactersToDeleteNextSaveCycle && m_charactersToDeleteThisSaveCycle) { @@ -375,8 +373,6 @@ void Persister::startSave(void) if (ConfigServerDatabase::getReportSaveTimes()) m_saveStartTime = Clock::timeMs(); - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -515,12 +511,9 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa { UNREF(serverId); - pad.lock(); - if (m_objectSnapshotMap.find(objectId)!=m_objectSnapshotMap.end()) { DEBUG_WARNING(true,("Database received multiple new object messages for object %s",objectId.getValueString().c_str())); - pad.unlock(); return; } @@ -575,8 +568,6 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa NOT_NULL(snap); snap->newObject(objectId, templateId, typeId); m_objectSnapshotMap[objectId]=snap; - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -589,9 +580,6 @@ void Persister::newObject(uint32 serverId, const NetworkId &objectId, int templa void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) { - - pad.lock(); - //TODO: This is a hack until we remove frame boundaries and have "end frame" messages from the game server. Apparently the game // server can split baselines across frame boundaries, so we can't assume we have all the data for a character when we hit a // frame bounday. @@ -601,8 +589,6 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) m_pendingCharacters.erase(chardata); m_newCharacterLock.erase(serverId); } - - pad.unlock(); } // ---------------------------------------------------------------------- @@ -613,65 +599,57 @@ void Persister::endBaselines(const NetworkId &objectId, uint32 serverId) void Persister::saveCompleted(Snapshot *completedSnapshot) { - pad.lock(); - - auto i=std::remove(m_savingSnapshots.begin(),m_savingSnapshots.end(),completedSnapshot); - if (i!=m_savingSnapshots.end()) - { - m_savingSnapshots.erase(i, m_savingSnapshots.end()); - - if (completedSnapshot != nullptr) { - delete completedSnapshot; - completedSnapshot = nullptr; + bool found = false; + for (auto i = m_savingSnapshots.begin(); i != m_savingSnapshots.end();) { + if (*i == completedSnapshot) { + i = m_savingSnapshots.erase(i); + found = true; + } else { + ++i; } + } - if (m_savingSnapshots.empty() && ConfigServerDatabase::getReportSaveTimes()) - { + if (m_savingSnapshots.empty()) + { + if (found && ConfigServerDatabase::getReportSaveTimes()) { int saveTime = Clock::timeMs() - m_saveStartTime; ++m_saveCount; m_totalSaveTime += saveTime; if (saveTime > m_maxSaveTime) m_maxSaveTime = saveTime; - DEBUG_REPORT_LOG(true,("Save completed in %i. (Average %i, max %i)\n", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); - LOG("SaveTimes",("Save completed in %i. (Average %i, max %i)", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); - - m_lastSaveTime = saveTime; + DEBUG_REPORT_LOG(true,("Save completed in %i. (Average %i, max %i)\n", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); + LOG("SaveTimes",("Save completed in %i. (Average %i, max %i)", saveTime, m_totalSaveTime/m_saveCount, m_maxSaveTime)); + + m_lastSaveTime = saveTime; } - if (m_savingSnapshots.empty()) - { - // message Central Server that the current save cycle is complete - GenericValueTypeMessage const saveCompleteMessage("DatabaseSaveComplete", ++m_saveCounter); - DatabaseProcess::getInstance().sendToCentralServer(saveCompleteMessage, true); - LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); - } + LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); - { - // set the last save completion time (for the monitoring program) - time_t theTime = time(0); - m_lastSaveCompletionTime = ctime(&theTime); - } + // TODO: so do we send this for the other snapshot type or not? hrmph + // message Central Server that the current save cycle is complete + GenericValueTypeMessage const saveCompleteMessage("DatabaseSaveComplete", ++m_saveCounter); + DatabaseProcess::getInstance().sendToCentralServer(saveCompleteMessage, true); + LOG("Database",("Sending DatabaseSaveComplete network message to Central.")); } - else - { - auto j=std::remove(m_savingCharacterSnapshots.begin(),m_savingCharacterSnapshots.end(),completedSnapshot); - - DEBUG_FATAL(i==m_savingCharacterSnapshots.end(),("Programmer bug: SaveCompleted() called with a snapshot that wasn't in m_savingSnapshots or m_savingCharacterSnapshots.")); - - if (j != m_savingCharacterSnapshots.end()) { - m_savingCharacterSnapshots.erase(j, m_savingCharacterSnapshots.end()); + + if (!found) { + for (auto i = m_savingCharacterSnapshots.begin(); i != m_savingCharacterSnapshots.end();) { + if (*i == completedSnapshot) { + i = m_savingCharacterSnapshots.erase(i); + found = true; + } else { + ++i; + } } - - if (completedSnapshot != nullptr) { - delete completedSnapshot; - completedSnapshot = nullptr; - } - + DEBUG_REPORT_LOG(ConfigServerDatabase::getReportSaveTimes(),("New character save completed\n")); } - pad.unlock(); + if (found && completedSnapshot != nullptr) { + delete completedSnapshot; + completedSnapshot = nullptr; + } } diff --git a/engine/server/library/serverDatabase/src/shared/Persister.h b/engine/server/library/serverDatabase/src/shared/Persister.h index 6f6ba2d6..31b09608 100755 --- a/engine/server/library/serverDatabase/src/shared/Persister.h +++ b/engine/server/library/serverDatabase/src/shared/Persister.h @@ -16,8 +16,6 @@ #include #include //TODO: remove when we clean up newCharacterLock hack -#include - #include "Unicode.h" #include "serverNetworkMessages/MessageToPayload.h" #include "sharedDatabaseInterface/DbModeQuery.h" @@ -99,8 +97,6 @@ class Persister : public MessageDispatch::Receiver DB::TaskQueue *m_newCharacterTaskQueue; private: - std::mutex pad; - struct PendingCharacter { uint32 stationId; diff --git a/engine/server/library/serverGame/src/shared/core/GameServer.cpp b/engine/server/library/serverGame/src/shared/core/GameServer.cpp index e760ae70..f48e37ad 100755 --- a/engine/server/library/serverGame/src/shared/core/GameServer.cpp +++ b/engine/server/library/serverGame/src/shared/core/GameServer.cpp @@ -4768,15 +4768,10 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse // ---------------------------------------------------------------------- // Set up the PlayerObject - - WARNING(true, ("Setting up the PlayerObject for %s", vrn.getName().c_str())); - ServerObject *playerServerObject = ServerWorld::createNewObject(ConfigServerGame::getPlayerObjectTemplate(), *newCharacterObject, false); PlayerObject *play = dynamic_cast(playerServerObject); if (play) { - WARNING(true, ("Player object is good...")); - play->setStationId(createMessage->getStationId()); play->setBornDate(); play->setSkillTemplate(createMessage->getSkillTemplate(), true); @@ -4811,8 +4806,6 @@ void GameServer::handleCharacterCreateNameVerification(const VerifyNameResponse WARNING_STRICT_FATAL(true,("Unable to create PlayerObject for new character %s.\n",newCharacterObject->getNetworkId().getValueString().c_str())); } - - WARNING(true, ("Setting scene and sending to DB for persist.")); newCharacterObject->setSceneIdOnThisAndContents(createMessage->getPlanetName()); // ---------------------------------------------------------------------- From 0995323201efb235496949112b87ed412d1c7f52 Mon Sep 17 00:00:00 2001 From: DarthArgus Date: Thu, 29 Dec 2016 08:53:48 +0000 Subject: [PATCH 3/3] move these to before and after as they were before the changes --- .../serverDatabase/src/shared/Snapshot.cpp | 28 ++----------------- .../src/shared/core/SwgSnapshot.cpp | 14 ++++++++++ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/engine/server/library/serverDatabase/src/shared/Snapshot.cpp b/engine/server/library/serverDatabase/src/shared/Snapshot.cpp index 163c2225..c61520c0 100755 --- a/engine/server/library/serverDatabase/src/shared/Snapshot.cpp +++ b/engine/server/library/serverDatabase/src/shared/Snapshot.cpp @@ -69,34 +69,10 @@ bool Snapshot::saveToDB(DB::Session *session) { NOT_NULL(session); - m_isBeingSaved = true; - - CustomStepListType::iterator step; - for (step=m_customStepList.begin(); step !=m_customStepList.end(); ++step) - { - NOT_NULL(*step); - if (!(*step)->beforePersist(session)) { - m_isBeingSaved = false; - return false; - } + if (m_timestamp != 0 && !saveTimestamp(session)) { + return false; } - if (m_timestamp!=0) - if (! saveTimestamp(session)) { - m_isBeingSaved = false; - return false; - } - - for (step=m_customStepList.begin(); step !=m_customStepList.end(); ++step) - { - NOT_NULL(*step); - if (!(*step)->afterPersist(session)){ - m_isBeingSaved = false; - return false; - } - } - - m_isBeingSaved = false; return true; } // ---------------------------------------------------------------------- diff --git a/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp b/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp index e6f04dd2..a111e70c 100755 --- a/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp +++ b/game/server/application/SwgDatabaseServer/src/shared/core/SwgSnapshot.cpp @@ -94,6 +94,13 @@ bool SwgSnapshot::saveToDB(DB::Session *session) { session->setAutoCommitMode(false); + for (auto step = m_customStepList.begin(); step !=m_customStepList.end(); ++step) + { + if (!(*step)->beforePersist(session)) { + return false; + } + } + // save all the buffers if (!(m_objectTableBuffer.save(session))) { return false; } if (!(m_battlefieldMarkerObjectBuffer.save(session))) { return false; } @@ -131,6 +138,13 @@ bool SwgSnapshot::saveToDB(DB::Session *session) { if (!(m_waypointBuffer.save(session))) { return false; } if (!(m_weaponObjectBuffer.save(session))) { return false; } + for (auto step = m_customStepList.begin(); step !=m_customStepList.end(); ++step) + { + if (!(*step)->afterPersist(session)){ + return false; + } + } + // save the parent class if (!(Snapshot::saveToDB(session))) { return false; }