From 4d58208676f3a2234d1c051c53775f8031560681 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 8 Feb 2019 09:57:11 +0100 Subject: [PATCH] File watcher: Reduce touch ignore duration On Linux and Windows the file watcher can't distinguish between changes that were caused by the process itself, like during a sync operation, and external changes. To work around that the client keeps a list of files it has touched and blocks notifications on these files for a bit. The duration of this block was originally and arbitrarily set at 15 seconds. During manual tests I regularly thought there was a bug when syncs didn't trigger, when the only problem was that my changes happened too close to a previous sync operation. This change reduces the duration to three seconds. I imagine that this is still enough. Also use std::chrono while at it. --- src/cmd/cmd.cpp | 2 +- src/libsync/propagateupload.cpp | 2 +- src/libsync/syncengine.cpp | 27 ++++++++++++++++++++++----- src/libsync/syncengine.h | 15 ++++++++++----- test/syncenginetestutils.h | 2 +- 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index 41d89f62b..bb6365aa2 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -497,7 +497,7 @@ int main(int argc, char **argv) } // much lower age than the default since this utility is usually made to be run right after a change in the tests - SyncEngine::minimumFileAgeForUpload = 0; + SyncEngine::minimumFileAgeForUpload = std::chrono::milliseconds(0); int restartCount = 0; restart_sync: diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 1908136fd..5f65d27c4 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -61,7 +61,7 @@ static bool fileIsStillChanging(const SyncFileItem &item) const QDateTime modtime = Utility::qDateTimeFromTime_t(item._modtime); const qint64 msSinceMod = modtime.msecsTo(QDateTime::currentDateTimeUtc()); - return msSinceMod < SyncEngine::minimumFileAgeForUpload + return std::chrono::milliseconds(msSinceMod) < SyncEngine::minimumFileAgeForUpload // if the mtime is too much in the future we *do* upload the file && msSinceMod > -10000; } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index c1613eb57..550f1347d 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -38,6 +38,7 @@ #include #include +#include #include #include @@ -58,10 +59,25 @@ namespace OCC { Q_LOGGING_CATEGORY(lcEngine, "nextcloud.sync.engine", QtInfoMsg) -static const int s_touchedFilesMaxAgeMs = 15 * 1000; bool SyncEngine::s_anySyncRunning = false; -qint64 SyncEngine::minimumFileAgeForUpload = 2000; +/** When the client touches a file, block change notifications for this duration (ms) + * + * On Linux and Windows the file watcher can't distinguish a change that originates + * from the client (like a download during a sync operation) and an external change. + * To work around that, all files the client touches are recorded and file change + * notifications for these are blocked for some time. This value controls for how + * long. + * + * Reasons this delay can't be very small: + * - it takes time for the change notification to arrive and to be processed by the client + * - some time could pass between the client recording that a file will be touched + * and its filesystem operation finishing, triggering the notification + */ +static const std::chrono::milliseconds s_touchedFilesMaxAgeMs(3 * 1000); + +// doc in header +std::chrono::milliseconds SyncEngine::minimumFileAgeForUpload(2000); SyncEngine::SyncEngine(AccountPtr account, const QString &localPath, const QString &remotePath, OCC::SyncJournalDb *journal) @@ -911,8 +927,9 @@ void SyncEngine::slotAddTouchedFile(const QString &fn) break; // Compare to our new QElapsedTimer instead of using elapsed(). // This avoids querying the current time from the OS for every loop. - if (now.msecsSinceReference() - first.key().msecsSinceReference() <= s_touchedFilesMaxAgeMs) { - // We found the first path younger than 15 second, keep the rest. + auto elapsed = std::chrono::milliseconds(now.msecsSinceReference() - first.key().msecsSinceReference()); + if (elapsed <= s_touchedFilesMaxAgeMs) { + // We found the first path younger than the maximum age, keep the rest. break; } @@ -934,7 +951,7 @@ bool SyncEngine::wasFileTouched(const QString &fn) const auto begin = _touchedFiles.constBegin(); for (auto it = _touchedFiles.constEnd(); it != begin; --it) { if ((it-1).value() == fn) - return (it-1).key().elapsed() <= s_touchedFilesMaxAgeMs; + return std::chrono::milliseconds((it-1).key().elapsed()) <= s_touchedFilesMaxAgeMs; } return false; } diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 49523eb47..9434ab5ac 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -87,12 +87,17 @@ public: SyncJournalDb *journal() const { return _journal; } QString localPath() const { return _localPath; } - /** - * Minimum age, in milisecond, of a file that can be uploaded. - * Files more recent than that are not going to be uploaeded as they are considered - * too young and possibly still changing + /** Duration in ms that uploads should be delayed after a file change + * + * In certain situations a file can be written to very regularly over a large + * amount of time. Copying a large file could take a while. A logfile could be + * updated every second. + * + * In these cases it isn't desirable to attempt to upload the "unfinished" file. + * To avoid that, uploads of files where the distance between the mtime and the + * current time is less than this duration are skipped. */ - static qint64 minimumFileAgeForUpload; // in ms + static std::chrono::milliseconds minimumFileAgeForUpload; /** * Control whether local discovery should read from filesystem or db. diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 8d836a3a0..d60d6dc3a 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -906,7 +906,7 @@ public: : _localModifier(_tempDir.path()) { // Needs to be done once - OCC::SyncEngine::minimumFileAgeForUpload = 0; + OCC::SyncEngine::minimumFileAgeForUpload = std::chrono::milliseconds(0); OCC::Logger::instance()->setLogFile("-"); QDir rootDir{_tempDir.path()};