From a75c7c49f011982aec86243f83a6d4066532391a Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 8 May 2022 21:28:43 +0200 Subject: [PATCH] Disable writes to navmeshdb on database is locked error Simultaneously writing to sqlite3 database is not possible. Process exclusively locks the database for this. Another process will fail to perform any request when database is locked. Alternatively it can wait. Handling this situation properly requires complexity that is not really needed. Users are not expected to run multiple openmw processes simultaneously using the same navmeshdb. Before this change running multiple openmw processes using the same navmeshdb can lead to a crash when first transaction fails to start because there is exception thrown and not catched. Remove use of explicit transactions from DbWorker. Handling all possible transaction states due to different errors brings unnecessary complexity. Initially they were introduced to increase time between flushes to disk. This makes sense for navmeshtool because of massive number of writes but for the engine this is not an issue. --- .../detournavigator/asyncnavmeshupdater.cpp | 19 ++++++- .../detournavigator/asyncnavmeshupdater.cpp | 53 ++++++------------- .../detournavigator/asyncnavmeshupdater.hpp | 3 +- 3 files changed, 33 insertions(+), 42 deletions(-) diff --git a/apps/openmw_test_suite/detournavigator/asyncnavmeshupdater.cpp b/apps/openmw_test_suite/detournavigator/asyncnavmeshupdater.cpp index 1f7125d1f2..06de335d50 100644 --- a/apps/openmw_test_suite/detournavigator/asyncnavmeshupdater.cpp +++ b/apps/openmw_test_suite/detournavigator/asyncnavmeshupdater.cpp @@ -262,6 +262,20 @@ namespace updater.post(mAgentHalfExtents, navMeshCacheItem, mPlayerTile, mWorldspace, changedTiles); updater.wait(mListener, WaitConditionType::allJobsDone); updater.stop(); + const std::set present { + TilePosition(-2, 0), + TilePosition(-1, -1), + TilePosition(-1, 0), + TilePosition(-1, 1), + TilePosition(0, -2), + TilePosition(0, -1), + TilePosition(0, 0), + TilePosition(0, 1), + TilePosition(0, 2), + TilePosition(1, -1), + TilePosition(1, 0), + TilePosition(1, 1), + }; for (int x = -5; x <= 5; ++x) for (int y = -5; y <= 5; ++y) { @@ -272,8 +286,9 @@ namespace [&] (const MeshSource& v) { return resolveMeshSource(*dbPtr, v); }); if (!objects.has_value()) continue; - EXPECT_FALSE(dbPtr->findTile(mWorldspace, tilePosition, serialize(mSettings.mRecast, *recastMesh, *objects)).has_value()) - << tilePosition.x() << " " << tilePosition.y(); + EXPECT_EQ(dbPtr->findTile(mWorldspace, tilePosition, serialize(mSettings.mRecast, *recastMesh, *objects)).has_value(), + present.find(tilePosition) != present.end()) + << tilePosition.x() << " " << tilePosition.y() << " present=" << (present.find(tilePosition) != present.end()); } } } diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 18bffae131..36c54a99b0 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -678,10 +678,10 @@ namespace DetourNavigator mHasJob.notify_all(); } - std::optional DbJobQueue::pop(std::chrono::steady_clock::duration timeout) + std::optional DbJobQueue::pop() { std::unique_lock lock(mMutex); - mHasJob.wait_for(lock, timeout, [&] { return mShouldStop || !mJobs.empty(); }); + mHasJob.wait(lock, [&] { return mShouldStop || !mJobs.empty(); }); if (mJobs.empty()) return std::nullopt; const JobIt job = mJobs.front(); @@ -752,45 +752,18 @@ namespace DetourNavigator void DbWorker::run() noexcept { - constexpr std::chrono::seconds transactionInterval(1); - auto transaction = mDb->startTransaction(Sqlite3::TransactionMode::Immediate); - auto start = std::chrono::steady_clock::now(); while (!mShouldStop) { try { - if (const auto job = mQueue.pop(transactionInterval)) + if (const auto job = mQueue.pop()) processJob(*job); - const auto now = std::chrono::steady_clock::now(); - if (mHasChanges && now - start > transactionInterval) - { - mHasChanges = false; - try - { - transaction.commit(); - } - catch (const std::exception& e) - { - Log(Debug::Error) << "DbWorker exception on commit: " << e.what(); - } - transaction = mDb->startTransaction(Sqlite3::TransactionMode::Immediate); - start = now; - } } catch (const std::exception& e) { Log(Debug::Error) << "DbWorker exception: " << e.what(); } } - if (mHasChanges) - try - { - transaction.commit(); - } - catch (const std::exception& e) - { - Log(Debug::Error) << "DbWorker exception on final commit: " << e.what(); - } } void DbWorker::processJob(JobIt job) @@ -804,10 +777,19 @@ namespace DetourNavigator catch (const std::exception& e) { Log(Debug::Error) << "DbWorker exception while processing job " << job->mId << ": " << e.what(); - if (std::string_view(e.what()).find("database or disk is full") != std::string_view::npos) + if (mWriteToDb) { - mWriteToDb = false; - Log(Debug::Warning) << "Writes to navmeshdb are disabled because file size limit is reached or disk is full"; + const std::string_view message(e.what()); + if (message.find("database or disk is full") != std::string_view::npos) + { + mWriteToDb = false; + Log(Debug::Warning) << "Writes to navmeshdb are disabled because file size limit is reached or disk is full"; + } + else if (message.find("database is locked") != std::string_view::npos) + { + mWriteToDb = false; + Log(Debug::Warning) << "Writes to navmeshdb are disabled to avoid concurrent writes from multiple processes"; + } } } }; @@ -833,11 +815,8 @@ namespace DetourNavigator Log(Debug::Debug) << "Serializing input for job " << job->mId; if (mWriteToDb) { - const ShapeId shapeId = mNextShapeId; const auto objects = makeDbRefGeometryObjects(job->mRecastMesh->getMeshSources(), [&] (const MeshSource& v) { return resolveMeshSource(*mDb, v, mNextShapeId); }); - if (shapeId != mNextShapeId) - mHasChanges = true; job->mInput = serialize(mRecastSettings, *job->mRecastMesh, objects); } else @@ -877,7 +856,6 @@ namespace DetourNavigator Log(Debug::Debug) << "Update db tile by job " << job->mId; job->mGeneratedNavMeshData->mUserId = cachedTileData->mTileId; mDb->updateTile(cachedTileData->mTileId, mVersion, serialize(*job->mGeneratedNavMeshData)); - mHasChanges = true; return; } @@ -893,6 +871,5 @@ namespace DetourNavigator mDb->insertTile(mNextTileId, job->mWorldspace, job->mChangedTile, mVersion, job->mInput, serialize(*job->mGeneratedNavMeshData)); ++mNextTileId; - mHasChanges = true; } } diff --git a/components/detournavigator/asyncnavmeshupdater.hpp b/components/detournavigator/asyncnavmeshupdater.hpp index dc6e9b5a81..ec8c849ca8 100644 --- a/components/detournavigator/asyncnavmeshupdater.hpp +++ b/components/detournavigator/asyncnavmeshupdater.hpp @@ -87,7 +87,7 @@ namespace DetourNavigator public: void push(JobIt job); - std::optional pop(std::chrono::steady_clock::duration timeout); + std::optional pop(); void update(TilePosition playerTile, int maxTiles); @@ -137,7 +137,6 @@ namespace DetourNavigator DbJobQueue mQueue; std::atomic_bool mShouldStop {false}; std::atomic_size_t mGetTileCount {0}; - bool mHasChanges = false; std::thread mThread; inline void run() noexcept;