loolwsd/DocumentBroker.cpp | 42 ++++++++++++++++++++---------------------- loolwsd/LOOLWSD.cpp | 9 --------- loolwsd/Storage.cpp | 6 +++--- loolwsd/Storage.hpp | 13 +++++++++---- loolwsd/protocol.txt | 2 +- 5 files changed, 33 insertions(+), 39 deletions(-)
New commits: commit 36fece8b07097d8e155f632abdf6c0f907550502 Author: Pranav Kant <[email protected]> Date: Sun Oct 23 17:35:46 2016 +0530 loolwsd: Separate WOPI load duration and check fileinfo duration Add both of them and send them as wopiloadduration to the client, if storage is WOPI. Change-Id: I5652d346d70f473624f03536469acf45470db742 diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index eeb3fd7..3a5e1be 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -237,19 +237,29 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI } // Lets load the document now - if (_storage->isLoaded()) + const bool loaded = _storage->isLoaded(); + if (!loaded) { - // Already loaded. Nothing to do. - return true; - } + const auto localPath = _storage->loadStorageFileToLocal(); + _uriJailed = Poco::URI(Poco::URI("file://"), localPath); + _filename = fileInfo._filename; - const auto localPath = _storage->loadStorageFileToLocal(); - _uriJailed = Poco::URI(Poco::URI("file://"), localPath); - _filename = fileInfo._filename; + // Use the local temp file's timestamp. + _lastFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified(); + _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot)); + } - // Use the local temp file's timestamp. - _lastFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified(); - _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot)); + // If its WOPI storage, send the duration that it took for WOPI calls + if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr) + { + // Get the time taken to load the file from storage + auto callDuration = dynamic_cast<WopiStorage*>(_storage.get())->getWopiLoadDuration(); + // Add the time taken to check file info + callDuration += fileInfo._callDuration; + const std::string msg = "stats: wopiloadduration " + std::to_string(callDuration.count()); + Log::trace("Sending to Client [" + msg + "]."); + it->second->sendTextFrame(msg); + } return true; } @@ -856,16 +866,6 @@ bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vecto return false; } -const std::chrono::duration<double> DocumentBroker::getStorageLoadDuration() const -{ - if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr) - { - return dynamic_cast<WopiStorage*>(_storage.get())->getWopiLoadDuration(); - } - - return std::chrono::duration<double>::zero(); -} - void DocumentBroker::childSocketTerminated() { if (!_childProcess->isAlive()) diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index 10b4e2f..3eefd6d 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -881,15 +881,6 @@ private: auto sessionsCount = docBroker->addSession(session); Log::trace(docKey + ", ws_sessions++: " + std::to_string(sessionsCount)); - // If its a WOPI host, return time taken to make calls to it - const auto storageCallDuration = docBroker->getStorageLoadDuration(); - if (storageCallDuration != std::chrono::duration<double>::zero()) - { - status = "stats: wopiloadduration " + std::to_string(storageCallDuration.count()); - Log::trace("Sending to Client [" + status + "]."); - ws->sendFrame(status.data(), (int) status.size()); - } - LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "NewSession: " + uri); // Let messages flow. diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp index a20691d..3a00066 100644 --- a/loolwsd/Storage.cpp +++ b/loolwsd/Storage.cpp @@ -306,9 +306,8 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic) Poco::StreamCopier::copyToString(rs, resMsg); const auto endTime = std::chrono::steady_clock::now(); - const std::chrono::duration<double> diff = (endTime - startTime); - _wopiLoadDuration += diff; - Log::debug("WOPI::CheckFileInfo returned: " + resMsg + ". Call duration: " + std::to_string(diff.count()) + "s"); + const std::chrono::duration<double> callDuration = (endTime - startTime); + Log::debug("WOPI::CheckFileInfo returned: " + resMsg + ". Call duration: " + std::to_string(callDuration.count()) + "s"); const auto index = resMsg.find_first_of('{'); if (index != std::string::npos) { @@ -331,6 +330,7 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic) // WOPI doesn't support file last modified time. _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName, canWrite}); + _fileInfo._callDuration = callDuration; return _fileInfo; } diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp index 163443c..6238a57 100644 --- a/loolwsd/Storage.hpp +++ b/loolwsd/Storage.hpp @@ -42,7 +42,8 @@ public: _size(size), _userId(userId), _userName(userName), - _canWrite(canWrite) + _canWrite(canWrite), + _callDuration(0) { } @@ -57,6 +58,10 @@ public: std::string _userId; std::string _userName; bool _canWrite; + + // Time it took to fetch fileinfo from storage + // Matters in case of external storage such as WOPI + std::chrono::duration<double> _callDuration; }; /// localStorePath the absolute root path of the chroot. @@ -164,11 +169,11 @@ public: bool saveLocalFileToStorage(const Poco::URI& uriPublic) override; - /// Total time taken for making WOPI calls during load (includes GetFileInfo calls) - const std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; } + /// Total time taken for making WOPI calls during load + std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; } private: - // Time spend in waiting for WOPI host during document load + // Time spend in loading the file from storage std::chrono::duration<double> _wopiLoadDuration; }; diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt index eb40f7a..560208f 100644 --- a/loolwsd/protocol.txt +++ b/loolwsd/protocol.txt @@ -334,7 +334,7 @@ redlinetablechanged: stats: <key> <value> - Contains statistical data. Eg: 'stats: wopiduration 5' means that + Contains statistical data. Eg: 'stats: wopiloadduration 5' means that wopi calls made in loading of document took 5 seconds. perm: <permission> commit 050bf4c65ec6901749da484cd568b175e98aa0ef Author: Pranav Kant <[email protected]> Date: Sun Oct 23 17:06:32 2016 +0530 loolwsd: Remove this comment - this is fixed now Change-Id: I032c7e4a1609b68882dba6cc48ebd3fb2d59b8f5 diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index 850d570..eeb3fd7 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -222,8 +222,6 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI if (_storage) { - // Set the username for the session - // TODO: security: Set the permission (readonly etc.) of the session here also const auto fileInfo = _storage->getFileInfo(uriPublic); if (!fileInfo.isValid()) { _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
