loolwsd/Log.cpp | 3 + loolwsd/TileCache.cpp | 14 +++---- loolwsd/Util.cpp | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++ loolwsd/Util.hpp | 14 +++++++ 4 files changed, 120 insertions(+), 7 deletions(-)
New commits: commit 891b942e7c1c4f41bfd138ece008d44d2ee87756 Author: Tor Lillqvist <[email protected]> Date: Tue Sep 27 15:48:32 2016 +0300 Handle disk full situations more gracefully Introduce new API in our Util namespace to save data to a file safely. The data is written to a temporary file in the same directory and after that has succeeded, it is renamed atomicaly to the intended name. If any step of the saving fails, neither the temporay file or the intended target (if one exists before) is left behind. Also add an API intended to alert the sysadmin in cases where their attention and action are required. This is not yet properly implemented. See FIXME comment for discussion. diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp index 232ca2b..1f9694c 100644 --- a/loolwsd/TileCache.cpp +++ b/loolwsd/TileCache.cpp @@ -151,11 +151,12 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const // Save to disk. const auto cachedName = (tileBeingRendered ? tileBeingRendered->getCacheName() : cacheFileName(tile)); + + // Ignore if we can't save the tile, things will work anyway, but slower. An error indication + // supposed to reach a sysadmin has been produced in that case. const auto fileName = _cacheDir + "/" + cachedName; - Log::trace() << "Saving cache tile: " << fileName << Log::end; - std::fstream outStream(fileName, std::ios::out); - outStream.write(data, size); - outStream.close(); + if (Util::saveDataToFileSafely(fileName, data, size, Poco::Message::PRIO_CRITICAL)) + Log::trace() << "Saved cache tile: " << fileName << Log::end; // Notify subscribers, if any. if (tileBeingRendered) @@ -274,9 +275,8 @@ void TileCache::saveRendering(const std::string& name, const std::string& dir, c const std::string fileName = dirName + "/" + name; - std::fstream outStream(fileName, std::ios::out); - outStream.write(data, size); - outStream.close(); + // Is failing to save a font as important as failing to save cached tiles? + Util::saveDataToFileSafely(fileName, data, size, Poco::Message::PRIO_CRITICAL); } std::unique_ptr<std::fstream> TileCache::lookupRendering(const std::string& name, const std::string& dir) diff --git a/loolwsd/Util.cpp b/loolwsd/Util.cpp index 0d137c4..3f25ae4 100644 --- a/loolwsd/Util.cpp +++ b/loolwsd/Util.cpp @@ -19,8 +19,11 @@ #include <atomic> #include <cassert> +#include <chrono> +#include <cstdio> #include <cstdlib> #include <cstring> +#include <fstream> #include <iomanip> #include <iostream> #include <mutex> @@ -33,6 +36,7 @@ #include <Poco/Exception.h> #include <Poco/Format.h> #include <Poco/Net/WebSocket.h> +#include <Poco/Message.h> #include <Poco/Process.h> #include <Poco/RandomStream.h> #include <Poco/TemporaryFile.h> @@ -102,6 +106,17 @@ namespace rng } } +namespace +{ + void alertSysadminOrLogNormally(const std::string& message, const std::string& tag, Poco::Message::Priority priority) + { + if (priority <= Poco::Message::PRIO_CRITICAL) + Util::alertSysadminWithoutSpamming(message, tag, 6); + else + Log::error(message + " Removing."); + } +} + namespace Util { std::string encodeId(const unsigned number, const int padding) @@ -142,6 +157,87 @@ namespace Util return std::getenv("DISPLAY") != nullptr; } + bool saveDataToFileSafely(std::string fileName, const char *data, size_t size, Poco::Message::Priority priority) + { + const auto tempFileName = fileName + ".temp"; + std::fstream outStream(tempFileName, std::ios::out); + + // If we can't create the file properly, just remove it + if (!outStream.good()) + { + alertSysadminOrLogNormally("Creating " + tempFileName + " failed, disk full?", "diskfull?", priority); + // Try removing both just in case + std::remove(tempFileName.c_str()); + std::remove(fileName.c_str()); + return false; + } + else + { + outStream.write(data, size); + if (!outStream.good()) + { + alertSysadminOrLogNormally("Writing to " + tempFileName + " failed, disk full?", "diskfull?", priority); + outStream.close(); + std::remove(tempFileName.c_str()); + std::remove(fileName.c_str()); + return false; + } + else + { + outStream.close(); + if (!outStream.good()) + { + alertSysadminOrLogNormally("Closing " + tempFileName + " failed, disk full?", "diskfull?", priority); + std::remove(tempFileName.c_str()); + std::remove(fileName.c_str()); + return false; + } + else + { + // Everything OK, rename the file to its proper name + if (std::rename(tempFileName.c_str(), fileName.c_str()) == 0) + { + Log::debug() << "Renaming " << tempFileName << " to " << fileName << " OK." << Log::end; + return true; + } + else + { + alertSysadminOrLogNormally("Renaming " + tempFileName + " to " + fileName + " failed, disk full?", "diskfull?", priority); + std::remove(tempFileName.c_str()); + std::remove(fileName.c_str()); + return false; + } + } + } + } + } + + void alertSysadminWithoutSpamming(const std::string& message, const std::string& tag, int maxMessagesPerDay) + { + static std::map<std::string, std::chrono::steady_clock::time_point> timeStamp; + + const auto now = std::chrono::steady_clock::now(); + const auto minInterval = std::chrono::seconds(24 * 60 * 60) / maxMessagesPerDay; + + if (timeStamp.find(tag) == timeStamp.end() || + timeStamp[tag] < now - minInterval) + { + // FIXME: Come up with something here that actually is a good way to notify the sysadmin + // so that this function actually does what it says on the tin. Is there some direct + // systemd way to log really important messages? And how to use that then conditionally + // on whether we actually are running under systemd, or just normally, for a developer + // or at some end-user that doesn't use systemd for LOOL. + + // Or should we extend our Log API to also have the CRITICAL level, and then get all our + // normal decorations in the logging also for this? On the other hand, it is a bit + // redundant actually to have our logging output timestamps, for instance, as systemd's + // logging mechanism automatically attaches timestamps to output from processes it runs + // anyway, doesn't it? + Log::logger().critical(message); + timeStamp[tag] = now; + } + } + const char *signalName(const int signo) { switch (signo) diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp index fb1bd43..3aab0ad 100644 --- a/loolwsd/Util.hpp +++ b/loolwsd/Util.hpp @@ -21,6 +21,7 @@ #include <Poco/File.h> #include <Poco/Net/WebSocket.h> +#include <Poco/Message.h> #include <Poco/Path.h> #include <Poco/Process.h> @@ -48,6 +49,19 @@ namespace Util bool windowingAvailable(); + // Save data to a file (overwriting an existing file if necessary) with checks for errors. Write + // to a temporary file in the same directory that is then atomically renamed to the desired name + // if everything goes well. In case of any error, both the destination file (if it already + // exists) and the temporary file (if was created, or existed already) are removed. Return true + // if everything succeeded. If priority is PRIO_CRITICAL or PRIO_FATAL, we will try to make sure + // an error message reaches a sysadmin. Such a message will be produced at most once every four + // hours during the runtime of the process to make it less likely they are ignored as spam. + bool saveDataToFileSafely(std::string fileName, const char *data, size_t size, Poco::Message::Priority priority); + + // Log the message with priority PRIO_CRITICAL. Don't log messages with the same tag more often + // than maxMessagesPerDay. + void alertSysadminWithoutSpamming(const std::string& message, const std::string& tag, int maxMessagesPerDay); + /// Assert that a lock is already taken. template <typename T> void assertIsLocked(T& lock) commit 8c404e700ee0cac042839055a075b486b3908954 Author: Tor Lillqvist <[email protected]> Date: Tue Sep 27 15:40:51 2016 +0300 Add FIXME about systemd logging mechanism considerations If we can find out that we are running under systemd, we probably shouldn't bother with any timestamps in our logging. Systemd does that, doesn't it? diff --git a/loolwsd/Log.cpp b/loolwsd/Log.cpp index 8575b49..1707f65 100644 --- a/loolwsd/Log.cpp +++ b/loolwsd/Log.cpp @@ -74,6 +74,9 @@ namespace Log static void getPrefix(char *buffer, const char* level) { + // FIXME: If running under systemd it is redundant to output timestamps, as those will be + // attached to messages that the systemd journalling mechanism picks up anyway, won't they? + Poco::Int64 usec = Poco::Timestamp().epochMicroseconds() - epochStart; const Poco::Int64 one_s = 1000000; _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
