[Libreoffice-commits] online.git: 2 commits - loolwsd/Log.cpp loolwsd/TileCache.cpp loolwsd/Util.cpp loolwsd/Util.hpp
Tor Lillqvist
tml at collabora.com
Tue Sep 27 12:56:19 UTC 2016
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 <tml at collabora.com>
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 <tml at collabora.com>
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;
More information about the Libreoffice-commits
mailing list