PropagateUpload: Model of remote quota, avoid some uploads #5537

When we see a 507 error, assume that quota is < uploaded size.
owncloud · Jul 12, 2017 · d08ed7d · d08ed7d
1 parent 14e75bf
commit d08ed7d
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 1 deletion.
diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h
@@ -351,6 +351,19 @@ class OwncloudPropagator : public QObject
     /** 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<QString, quint64> _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
@@ -197,6 +197,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<quint64>::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) {
@@ -543,8 +554,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();
@@ -608,10 +629,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
@@ -465,6 +465,55 @@ 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._maximumNetworkJobs = 1;
+        syncOptions._maximumTransferJobs = 1;
+        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)