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.

diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp
index 41d89f62b179a4e992a1254b969999901fa422ba..bb6365aa218bdb4aaff3acd6f66541e21c43571d 100644 (file)
--- 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 1908136fdef42535ab19023545a5cd77c062ebec..5f65d27c41d6a16ab8ebf95fdbad6f458468b679 100644 (file)
--- 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 c1613eb575f809006110697472c330aff8b70234..550f1347d118eec29f4cb74eab446401b57a5771 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -38,6 +38,7 @@
 
 #include <climits>
 #include <cassert>
+#include <chrono>
 
 #include <QCoreApplication>
 #include <QSslSocket>
@@ -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 49523eb4777fec5243f76ac1fa1a22481be4afe5..9434ab5acedacbb76f95f05eda931556dbaa47ca 100644 (file)
--- 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 8d836a3a0910cbb803ac2c127db23d52560f1533..d60d6dc3a4d1b0300c59a9693fdd2fcf65ca2764 100644 (file)
--- 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()};