From 9788055147ae6e76f1354485abb678f27968dbb8 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 29 Oct 2015 09:35:42 +0100 Subject: [PATCH] Propagator: Add blacklisting of disk space errors #2939 --- src/libsync/owncloudpropagator.cpp | 6 +++--- src/libsync/propagatedownload.cpp | 1 + src/libsync/syncfileitem.h | 10 +++++++++- src/libsync/syncjournalfilerecord.cpp | 14 +++++++++----- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 79a91203d..ae83b32c1 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -87,11 +87,11 @@ int OwncloudPropagator::maximumActiveJob() return max; } -/** Updates or creates a blacklist entry for the given item. +/** Updates, creates or removes a blacklist entry for the given item. * * Returns whether the file is in the blacklist now. */ -static bool blacklist(SyncJournalDb* journal, const SyncFileItem& item) +static bool blacklistCheck(SyncJournalDb* journal, const SyncFileItem& item) { SyncJournalErrorBlacklistRecord oldEntry = journal->errorBlacklistEntry(item._file); SyncJournalErrorBlacklistRecord newEntry = SyncJournalErrorBlacklistRecord::update(oldEntry, item); @@ -132,7 +132,7 @@ void PropagateItemJob::done(SyncFileItem::Status status, const QString &errorStr // do not blacklist in case of soft error or fatal error. break; case SyncFileItem::NormalError: - if (blacklist(_propagator->_journal, *_item) && _item->_hasBlacklistEntry) { + if (blacklistCheck(_propagator->_journal, *_item) && _item->_hasBlacklistEntry) { // do not error if the item was, and continues to be, blacklisted status = SyncFileItem::FileIgnored; _item->_errorString.prepend(tr("Continue blacklisting:") + " "); diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index efb5f6630..b52126700 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -359,6 +359,7 @@ void PropagateDownloadFileQNAM::start() // If there's not enough space to fully download this file, stop. const auto diskSpaceResult = _propagator->diskSpaceCheck(); if (diskSpaceResult == OwncloudPropagator::DiskSpaceFailure) { + _item->_errorMayBeBlacklisted = true; done(SyncFileItem::NormalError, tr("The download would reduce free disk space below %1").arg( Utility::octetsToString(freeSpaceLimit()))); diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index e6b18359f..bb2a1af99 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -64,7 +64,8 @@ public: }; SyncFileItem() : _type(UnknownType), _direction(None), _isDirectory(false), - _serverHasIgnoredFiles(false), _hasBlacklistEntry(false), _status(NoStatus), + _serverHasIgnoredFiles(false), _hasBlacklistEntry(false), + _errorMayBeBlacklisted(false), _status(NoStatus), _isRestoration(false), _should_update_metadata(false), _httpErrorCode(0), _requestDuration(0), _affectedItems(1), _instruction(CSYNC_INSTRUCTION_NONE), _modtime(0), _size(0), _inode(0) @@ -137,6 +138,13 @@ public: /// without the status being FileIgnored. bool _hasBlacklistEntry BITFIELD(1); + /** If true and NormalError, this error may be blacklisted + * + * Note that non-local errors (httpErrorCode!=0) may also be + * blacklisted independently of this flag. + */ + bool _errorMayBeBlacklisted BITFIELD(1); + // Variables useful to report to the user Status _status BITFIELD(4); bool _isRestoration BITFIELD(1); // The original operation was forbidden, and this is a restoration diff --git a/src/libsync/syncjournalfilerecord.cpp b/src/libsync/syncjournalfilerecord.cpp index c3c561378..625706251 100644 --- a/src/libsync/syncjournalfilerecord.cpp +++ b/src/libsync/syncjournalfilerecord.cpp @@ -108,12 +108,16 @@ SyncJournalErrorBlacklistRecord SyncJournalErrorBlacklistRecord::update( const SyncJournalErrorBlacklistRecord& old, const SyncFileItem& item) { SyncJournalErrorBlacklistRecord entry; - if (item._httpErrorCode == 0 // Do not blacklist local errors. (#1985) + bool mayBlacklist = + item._errorMayBeBlacklisted // explicitly flagged for blacklisting + || (item._httpErrorCode != 0 // or non-local error #ifdef OWNCLOUD_5XX_NO_BLACKLIST - || item._httpErrorCode / 100 == 5 // In this configuration, never blacklist error 5xx + && item._httpErrorCode / 100 != 5 // In this configuration, never blacklist error 5xx #endif - ) { - qDebug() << "This error is not blacklisted " << item._httpErrorCode; + ); + + if (!mayBlacklist) { + qDebug() << "This error is not blacklisted " << item._httpErrorCode << item._errorMayBeBlacklisted; return entry; } @@ -126,7 +130,7 @@ SyncJournalErrorBlacklistRecord SyncJournalErrorBlacklistRecord::update( entry._lastTryEtag = item._etag; entry._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTime()); // The factor of 5 feels natural: 25s, 2 min, 10 min, ~1h, ~5h, ~24h - entry._ignoreDuration = qMin(qMax(minBlacklistTime, old._ignoreDuration * 5), maxBlacklistTime); + entry._ignoreDuration = qBound(minBlacklistTime, old._ignoreDuration * 5, maxBlacklistTime); entry._file = item._file; if( item._httpErrorCode == 403 || item._httpErrorCode == 413 || item._httpErrorCode == 415 ) { -- 2.30.2