From e306f4611c4a1f51a2a30fb4861db8553683ff2e Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 6 Dec 2016 13:18:59 +0100 Subject: [PATCH] Reschedule a folder regularly with some delay for some errors Like "folder doesn't exist" - such that we will detect when the folder becomes available and start syncing. See #5317 --- src/cmd/cmd.cpp | 2 +- src/gui/folder.cpp | 6 +++--- src/gui/folderman.cpp | 22 ++++++++++++---------- src/libsync/syncengine.cpp | 14 +++++++++----- src/libsync/syncengine.h | 13 ++++++++++--- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index e8911b1de..d4a9fc581 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -507,7 +507,7 @@ restart_sync: app.exec(); - if (engine.isAnotherSyncNeeded()) { + if (engine.isAnotherSyncNeeded() != NoFollowUpSync) { if (restartCount < options.restartTimes) { restartCount++; qDebug() << "Restarting Sync, because another sync is needed" << restartCount; diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index ce02e0aa8..28456b947 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -823,7 +823,7 @@ void Folder::slotSyncFinished(bool success) _fileLog->finish(); bubbleUpSyncResult(); - bool anotherSyncNeeded = _engine->isAnotherSyncNeeded(); + auto anotherSyncNeeded = _engine->isAnotherSyncNeeded(); if (_csyncError) { _syncResult.setStatus(SyncResult::Error); @@ -874,7 +874,7 @@ void Folder::slotSyncFinished(bool success) _timeSinceLastSyncDone.restart(); // Increment the follow-up sync counter if necessary. - if (anotherSyncNeeded) { + if (anotherSyncNeeded == ImmediateFollowUp) { _consecutiveFollowUpSyncs++; qDebug() << "another sync was requested by the finished sync, this has" << "happened" << _consecutiveFollowUpSyncs << "times"; @@ -883,7 +883,7 @@ void Folder::slotSyncFinished(bool success) } // Maybe force a follow-up sync to take place, but only a couple of times. - if (anotherSyncNeeded && _consecutiveFollowUpSyncs <= 3) + if (anotherSyncNeeded == ImmediateFollowUp && _consecutiveFollowUpSyncs <= 3) { // Sometimes another sync is requested because a local file is still // changing, so wait at least a small amount of time before syncing diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index a1f6b209d..9715312ba 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -833,18 +833,20 @@ void FolderMan::slotScheduleFolderByTime() continue; } - // Retry a couple of times after failure - bool syncAgainAfterFail = f->consecutiveFailingSyncs() > 0 && f->consecutiveFailingSyncs() < 3; - qint64 syncAgainAfterFailDelay = 10 * 1000; // 10s for the first retry-after-fail + // Retry a couple of times after failure; or regularly if requested + bool syncAgain = + (f->consecutiveFailingSyncs() > 0 && f->consecutiveFailingSyncs() < 3) + || f->syncEngine().isAnotherSyncNeeded() == DelayedFollowUp; + qint64 syncAgainDelay = 10 * 1000; // 10s for the first retry-after-fail if (f->consecutiveFailingSyncs() > 1) - syncAgainAfterFailDelay = 60 * 1000; // 60s for each further attempt - if (syncAgainAfterFail - && msecsSinceSync > syncAgainAfterFailDelay) { + syncAgainDelay = 60 * 1000; // 60s for each further attempt + if (syncAgain + && msecsSinceSync > syncAgainDelay) { qDebug() << "** scheduling folder" << f->alias() - << "because the last" - << f->consecutiveFailingSyncs() << "syncs failed, last status:" - << f->syncResult().statusString() - << "time since last sync:" << msecsSinceSync; + << ", the last" << f->consecutiveFailingSyncs() << "syncs failed" + << ", anotherSyncNeeded" << f->syncEngine().isAnotherSyncNeeded() + << ", last status:" << f->syncResult().statusString() + << ", time since last sync:" << msecsSinceSync; scheduleFolder(f); continue; diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 8c6488ad8..33357e15d 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -73,7 +73,7 @@ SyncEngine::SyncEngine(AccountPtr account, const QString& localPath, , _downloadLimit(0) , _newBigFolderSizeLimit(-1) , _checksum_hook(journal) - , _anotherSyncNeeded(false) + , _anotherSyncNeeded(NoFollowUpSync) { qRegisterMetaType("SyncFileItem"); qRegisterMetaType("SyncFileItem::Status"); @@ -695,12 +695,13 @@ void SyncEngine::startSync() Q_ASSERT(!_syncRunning); s_anySyncRunning = true; _syncRunning = true; - _anotherSyncNeeded = false; + _anotherSyncNeeded = NoFollowUpSync; _clearTouchedFilesTimer.stop(); _progressInfo->reset(); if (!QDir(_localPath).exists()) { + _anotherSyncNeeded = DelayedFollowUp; // No _tr, it should only occur in non-mirall emit csyncError("Unable to find local sync folder."); finalize(false); @@ -714,6 +715,7 @@ void SyncEngine::startSync() qDebug() << "There are" << freeBytes << "bytes available at" << _localPath << "and at least" << minFree << "are required"; if (freeBytes < minFree) { + _anotherSyncNeeded = DelayedFollowUp; emit csyncError(tr("Only %1 are available, need at least %2 to start", "Placeholders are postfixed with file sizes using Utility::octetsToString()").arg( Utility::octetsToString(freeBytes), @@ -1050,7 +1052,9 @@ void SyncEngine::slotItemCompleted(const SyncFileItem &item, const PropagatorJob void SyncEngine::slotFinished(bool success) { - _anotherSyncNeeded = _anotherSyncNeeded || _propagator->_anotherSyncNeeded; + if (_propagator->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) { + _anotherSyncNeeded = ImmediateFollowUp; + } if (success) { _journal->setDataFingerprint(_discoveryMainThread->_dataFingerprint); @@ -1173,7 +1177,7 @@ void SyncEngine::checkForPermission() // be restored. Do that in the next sync by not considering as a rename // but delete and upload. It will then be restored if needed. _journal->avoidRenamesOnNextSync((*it)->_file); - _anotherSyncNeeded = true; + _anotherSyncNeeded = ImmediateFollowUp; qDebug() << "Moving of " << (*it)->_file << " canceled because no permission to add parent folder"; } (*it)->_instruction = CSYNC_INSTRUCTION_ERROR; @@ -1330,7 +1334,7 @@ void SyncEngine::checkForPermission() // At this point we would need to go back to the propagate phase on both remote to take // the decision. _journal->avoidRenamesOnNextSync((*it)->_file); - _anotherSyncNeeded = true; + _anotherSyncNeeded = ImmediateFollowUp; if ((*it)->_isDirectory) { diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 42349d69f..d29c433b1 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -49,6 +49,13 @@ class SyncJournalDb; class OwncloudPropagator; class PropagatorJob; +enum AnotherSyncNeeded +{ + NoFollowUpSync, + ImmediateFollowUp, // schedule this again immediately (limited amount of times) + DelayedFollowUp // regularly schedule this folder again (around 1/minute, unlimited) +}; + /** * @brief The SyncEngine class * @ingroup libsync @@ -82,8 +89,8 @@ public: Utility::StopWatch &stopWatch() { return _stopWatch; } SyncFileStatusTracker &syncFileStatusTracker() { return *_syncFileStatusTracker; } - /* Return true if we detected that another sync is needed to complete the sync */ - bool isAnotherSyncNeeded() { return _anotherSyncNeeded; } + /* Returns whether another sync is needed to complete the sync */ + AnotherSyncNeeded isAnotherSyncNeeded() { return _anotherSyncNeeded; } /** Get the ms since a file was touched, or -1 if it wasn't. * @@ -259,7 +266,7 @@ private: /// Hook for computing checksums from csync_update CSyncChecksumHook _checksum_hook; - bool _anotherSyncNeeded; + AnotherSyncNeeded _anotherSyncNeeded; /** Stores the time since a job touched a file. */ QHash _touchedFiles; -- 2.30.2