From: Christian Kamm Date: Wed, 12 Jul 2017 07:58:15 +0000 (+0200) Subject: PropagateUpload: Model of remote quota, avoid some uploads #5537 X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~701^2~104 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=0e8ce9c3db2ae14c9714f730df9771a31265dc86;p=nextcloud-desktop.git PropagateUpload: Model of remote quota, avoid some uploads #5537 When we see a 507 error, assume that quota is < uploaded size. --- diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 21da3d72c..093fd8aac 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -350,6 +350,19 @@ public: /** We detected that another sync is required after this one */ bool _anotherSyncNeeded; + /** Per-folder quota guesses. + * + * This starts out empty. When an upload in a folder fails due to insufficent + * remote quota, the quota guess is updated to be attempted_size-1 at maximum. + * + * Note that it will usually just an upper limit for the actual quota - but + * since the quota on the server might change at any time it can sometimes be + * wrong in the other direction as well. + * + * This allows skipping of uploads that have a very high likelihood of failure. + */ + QHash _folderQuota; + /* the maximum number of jobs using bandwidth (uploads or downloads, in parallel) */ int maximumActiveTransferJob(); diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 73002c427..f72d36ca7 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -176,6 +176,17 @@ void PropagateUploadFileCommon::start() return; } + // Check if we believe that the upload will fail due to remote quota limits + const quint64 quotaGuess = propagator()->_folderQuota.value( + QFileInfo(_item->_file).path(), std::numeric_limits::max()); + if (_item->_size > quotaGuess) { + // Necessary for blacklisting logic + _item->_httpErrorCode = 507; + emit propagator()->insufficientRemoteStorage(); + done(SyncFileItem::DetailError, tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size))); + return; + } + propagator()->_activeJobList.append(this); if (!_deleteExisting) { @@ -522,8 +533,18 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job) SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode, &propagator()->_anotherSyncNeeded); + // Insufficient remote storage. if (_item->_httpErrorCode == 507) { - // Insufficient remote storage. + // Update the quota expectation + const auto path = QFileInfo(_item->_file).path(); + auto quotaIt = propagator()->_folderQuota.find(path); + if (quotaIt != propagator()->_folderQuota.end()) { + quotaIt.value() = qMin(quotaIt.value(), _item->_size - 1); + } else { + propagator()->_folderQuota[path] = _item->_size - 1; + } + + // Set up the error status = SyncFileItem::DetailError; errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size)); emit propagator()->insufficientRemoteStorage(); @@ -587,10 +608,17 @@ void PropagateUploadFileCommon::finalize() { _finished = true; + // Update the quota, if known + auto quotaIt = propagator()->_folderQuota.find(QFileInfo(_item->_file).path()); + if (quotaIt != propagator()->_folderQuota.end()) + quotaIt.value() -= _item->_size; + + // Update the database entry if (!propagator()->_journal->setFileRecord(SyncJournalFileRecord(*_item, propagator()->getFilePath(_item->_file)))) { done(SyncFileItem::FatalError, tr("Error writing metadata to the database")); return; } + // Remove from the progress database: propagator()->_journal->setUploadInfo(_item->_file, SyncJournalDb::UploadInfo()); propagator()->_journal->commit("upload file start"); diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index cfac432f9..716bc5329 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -465,6 +465,54 @@ private slots: QVERIFY(fakeFolder.syncOnce()); } + + /** + * Checks whether subsequent large uploads are skipped after a 507 error + */ + void testInsufficientRemoteStorage() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + + // Disable parallel uploads + SyncOptions syncOptions; + syncOptions._parallelNetworkJobs = false; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + + // Produce an error based on upload size + int remoteQuota = 1000; + int n507 = 0, nPUT = 0; + auto parent = new QObject; + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request) -> QNetworkReply * { + if (op == QNetworkAccessManager::PutOperation) { + nPUT++; + if (request.rawHeader("OC-Total-Length").toInt() > remoteQuota) { + n507++; + return new FakeErrorReply(op, request, parent, 507); + } + } + return nullptr; + }); + + fakeFolder.localModifier().insert("A/big", 800); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nPUT, 1); + QCOMPARE(n507, 0); + + nPUT = 0; + fakeFolder.localModifier().insert("A/big1", 500); // ok + fakeFolder.localModifier().insert("A/big2", 1200); // 507 (quota guess now 1199) + fakeFolder.localModifier().insert("A/big3", 1200); // skipped + fakeFolder.localModifier().insert("A/big4", 1500); // skipped + fakeFolder.localModifier().insert("A/big5", 1100); // 507 (quota guess now 1099) + fakeFolder.localModifier().insert("A/big6", 900); // ok (quota guess now 199) + fakeFolder.localModifier().insert("A/big7", 200); // skipped + fakeFolder.localModifier().insert("A/big8", 199); // ok (quota guess now 0) + + fakeFolder.localModifier().insert("B/big8", 1150); // 507 + QVERIFY(!fakeFolder.syncOnce()); + QCOMPARE(nPUT, 6); + QCOMPARE(n507, 3); + } }; QTEST_GUILESS_MAIN(TestSyncEngine)