Upload: Auto limit max chunk size on HTTP-413

* Using Account to remember max and last dynamic chunk size * Start sensing max upload size at 100mb (on first failure) * Handle connection close error like HTTP-413 if upload-size > 100mb * Updated tests that relied on boundless SyncOptions Signed-off-by: Juergen Kellerer <[email protected]>
nextcloud · Sep 22, 2023 · dc7bc44 · dc7bc44
1 parent 395edd9
commit dc7bc44
Show file tree

Hide file tree

Showing 18 changed files with 425 additions and 96 deletions.
diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp
@@ -531,7 +531,7 @@ int main(int argc, char **argv)
 
     SyncOptions opt;
     opt.fillFromEnvironmentVariables();
-    opt.verifyChunkSizes();
+    opt.fillFromAccount(account);
     SyncEngine engine(account, options.source_dir, opt, folder, &db);
     engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles);
     engine.setNetworkLimits(options.uplimit, options.downlimit);

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
@@ -28,6 +28,7 @@
 #include "syncresult.h"
 #include "clientproxy.h"
 #include "syncengine.h"
+#include "owncloudpropagator.h"
 #include "syncrunfilelog.h"
 #include "socketapi/socketapi.h"
 #include "theme.h"
@@ -1096,16 +1097,15 @@ SyncOptions Folder::initializeSyncOptions() const
     opt._confirmExternalStorage = cfgFile.confirmExternalStorage();
     opt._moveFilesToTrash = cfgFile.moveToTrash();
     opt._vfs = _vfs;
-    opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6;
 
     // Chunk V2: Size of chunks must be between 5MB and 5GB, except for the last chunk which can be smaller
     opt.setMinChunkSize(cfgFile.minChunkSize());
     opt.setMaxChunkSize(cfgFile.maxChunkSize());
-    opt._initialChunkSize = ::qBound(opt.minChunkSize(), cfgFile.chunkSize(), opt.maxChunkSize());
+    opt.setInitialChunkSize(cfgFile.chunkSize());
     opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration();
 
     opt.fillFromEnvironmentVariables();
-    opt.verifyChunkSizes();
+    opt.fillFromAccount(_accountState->account());
 
     return opt;
 }

diff --git a/src/gui/socketapi/socketuploadjob.cpp b/src/gui/socketapi/socketuploadjob.cpp
@@ -55,7 +55,7 @@ SocketUploadJob::SocketUploadJob(const QSharedPointer<SocketApiJobV2> &job)
 
     SyncOptions opt;
     opt.fillFromEnvironmentVariables();
-    opt.verifyChunkSizes();
+    opt.fillFromAccount(account->account());
     _engine = new SyncEngine(account->account(), Utility::trailingSlashPath(_localPath), opt, _remotePath, _db);
     _engine->setParent(_db);
 

diff --git a/src/libsync/account.h b/src/libsync/account.h
@@ -16,6 +16,7 @@
 #ifndef SERVERCONNECTION_H
 #define SERVERCONNECTION_H
 
+#include <QAtomicInteger>
 #include <QByteArray>
 #include <QUrl>
 #include <QNetworkCookie>
@@ -276,6 +277,24 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
     bool isHttp2Supported() { return _http2Supported; }
     void setHttp2Supported(bool value) { _http2Supported = value; }
 
+    /** Max allowed request size in bytes that the server accepts. Is <= 0 if not limited */
+    qint64 getMaxRequestSize() const { return _maxRequestSize; }
+    void setMaxRequestSize(qint64 value) { _maxRequestSize = value; }
+    void setMaxRequestSizeIfLower(qint64 value) {
+        while (true) {
+            const auto size = _maxRequestSize;
+
+            if (size > 0 && value >= size)
+                break;
+            if (_maxRequestSize.testAndSetOrdered(size, value))
+                break;
+        }
+    }
+
+    /** Last successful chunk size in bytes, used to speedup auto-sensing. */
+    qint64 getLastChunkSize() const { return _lastChunkSize; }
+    void setLastChunkSize(qint64 value) { _lastChunkSize = value; }
+
     void clearCookieJar();
     void lendCookieJarTo(QNetworkAccessManager *guest);
     QString cookieJarPath();
@@ -421,6 +440,8 @@ private slots:
     QSharedPointer<QNetworkAccessManager> _am;
     QScopedPointer<AbstractCredentials> _credentials;
     bool _http2Supported = false;
+    QAtomicInteger<qint64> _maxRequestSize = -1;
+    QAtomicInteger<qint64> _lastChunkSize = -1;
 
     /// Certificates that were explicitly rejected by the user
     QList<QSslCertificate> _rejectedCertificates;

diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp
@@ -15,6 +15,7 @@
 #include "config.h"
 
 #include "configfile.h"
+#include "syncoptions.h"
 #include "theme.h"
 #include "version.h"
 #include "common/utility.h"
@@ -246,25 +247,25 @@ int ConfigFile::timeout() const
 qint64 ConfigFile::chunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(chunkSizeC), 10LL * 1000LL * 1000LL).toLongLong(); // default to 10 MB
+    return settings.value(QLatin1String(chunkSizeC), SyncOptions::defaultChunkSize).toLongLong(); // default to 10 MB
 }
 
 qint64 ConfigFile::maxChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(maxChunkSizeC), 5LL * 1000LL * 1000LL * 1000LL).toLongLong(); // default to 5000 MB
+    return settings.value(QLatin1String(maxChunkSizeC), SyncOptions::chunkV2MaxChunkSize).toLongLong(); // default to 5 GB
 }
 
 qint64 ConfigFile::minChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(minChunkSizeC), 5LL * 1000LL * 1000LL).toLongLong(); // default to 5 MB
+    return settings.value(QLatin1String(minChunkSizeC), SyncOptions::chunkV2MinChunkSize).toLongLong(); // default to 5 MB
 }
 
 chrono::milliseconds ConfigFile::targetChunkUploadDuration() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return millisecondsValue(settings, targetChunkUploadDurationC, chrono::minutes(1));
+    return millisecondsValue(settings, targetChunkUploadDurationC, SyncOptions::chunkV2TargetChunkUploadDuration);
 }
 
 void ConfigFile::setOptionalServerNotifications(bool show)

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
@@ -395,8 +395,8 @@ std::unique_ptr<PropagateUploadFileCommon> OwncloudPropagator::createUploadJob(S
 {
     auto job = std::unique_ptr<PropagateUploadFileCommon>{};
 
-    if (item->_size > syncOptions()._initialChunkSize && account()->capabilities().chunkingNg()) {
-        // Item is above _initialChunkSize, thus will be classified as to be chunked
+    if (item->_size > syncOptions().initialChunkSize() && account()->capabilities().chunkingNg()) {
+        // Item is above initialChunkSize(), thus will be classified as to be chunked
         job = std::make_unique<PropagateUploadFileNG>(this, item);
     } else {
         job = std::make_unique<PropagateUploadFileV1>(this, item);
@@ -706,7 +706,8 @@ const SyncOptions &OwncloudPropagator::syncOptions() const
 void OwncloudPropagator::setSyncOptions(const SyncOptions &syncOptions)
 {
     _syncOptions = syncOptions;
-    _chunkSize = syncOptions._initialChunkSize;
+    _syncOptions.fillFromAccount(account());
+    _chunkSize = _syncOptions.initialChunkSize();
 }
 
 bool OwncloudPropagator::localFileNameClash(const QString &relFile)

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
@@ -676,6 +676,49 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
     SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode,
         &propagator()->_anotherSyncNeeded, replyContent);
 
+    // Request entity to large: Chunk size is larger than supported by the server
+    if (status == SyncFileItem::NormalError && _item->_requestBodySize > 0) {
+        const auto opts = propagator()->syncOptions();
+        const auto requestSize = _item->_requestBodySize;
+
+        // Check if remote host closed the connection after an upload larger than SyncOptions::maxChunkSizeAfterFailure
+        const bool isConnectionClosedOnLargeUpload = job->reply()->error() == QNetworkReply::RemoteHostClosedError
+                                                     && requestSize > SyncOptions::maxChunkSizeAfterFailure;
+
+        if (isConnectionClosedOnLargeUpload) {
+            qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" 
+                                        << Utility::octetsToString(requestSize) << "."
+                                        << "Treating the error like HTTP-413 (request entity to large)";
+        }
+
+        // Calculate new upload limit and set it in propagator()->account() 
+        // Sent bytes is no indicator how much is allowed by the server as requests must be consumed fully.
+        if (_item->_httpErrorCode == 413 || isConnectionClosedOnLargeUpload) {
+            const auto maxRequestSize = opts.toValidChunkSize(
+                requestSize > SyncOptions::maxChunkSizeAfterFailure
+                    ? SyncOptions::maxChunkSizeAfterFailure
+                    : requestSize / 2
+            );
+
+            // Set new upload limit
+            propagator()->account()->setMaxRequestSizeIfLower(maxRequestSize);
+
+            // Apply to this propagator (limit is applied in setSyncOptions())
+            propagator()->setSyncOptions(opts);
+            const auto adjustedOpts = propagator()->syncOptions();
+
+            // Trigger retry
+            if (adjustedOpts.maxChunkSize() < requestSize) {
+                propagator()->_anotherSyncNeeded = true;
+                status = SyncFileItem::SoftError;
+            }
+
+            errorString = tr("Upload of %1 exceeds the max upload size allowed by the server. Reducing max upload size to %2")
+                            .arg(Utility::octetsToString(requestSize))
+                            .arg(Utility::octetsToString(adjustedOpts.maxChunkSize()));
+        }
+    }
+
     // Insufficient remote storage.
     if (_item->_httpErrorCode == 507) {
         // Update the quota expectation

diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h
@@ -351,7 +351,7 @@ class PropagateUploadFileV1 : public PropagateUploadFileCommon
     [[nodiscard]] qint64 chunkSize() const {
         // Old chunking does not use dynamic chunking algorithm, and does not adjusts the chunk size respectively,
         // thus this value should be used as the one classifing item to be chunked
-        return propagator()->syncOptions()._initialChunkSize;
+        return propagator()->syncOptions().initialChunkSize();
     }
 
 public:

diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp
@@ -403,6 +403,7 @@ void PropagateUploadFileNG::slotPutFinished()
     if (err != QNetworkReply::NoError) {
         _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         _item->_requestId = job->requestId();
+        _item->_requestBodySize = job->device()->size();
         commonErrorHandling(job);
         return;
     }
@@ -413,26 +414,31 @@ void PropagateUploadFileNG::slotPutFinished()
     //
     // Dynamic chunk sizing is enabled if the server configured a
     // target duration for each chunk upload.
-    auto targetDuration = propagator()->syncOptions()._targetChunkUploadDuration;
-    if (targetDuration.count() > 0) {
-        auto uploadTime = ++job->msSinceStart(); // add one to avoid div-by-zero
-        qint64 predictedGoodSize = (_currentChunkSize * targetDuration) / uploadTime;
+    if (const auto opts = propagator()->syncOptions(); opts.isDynamicChunkSize()) {
+        const auto uploadTime = job->msSinceStart();
+        const auto predictedGoodSize = opts.predictedGoodChunkSize(_currentChunkSize, uploadTime);
 
         // The whole targeting is heuristic. The predictedGoodSize will fluctuate
         // quite a bit because of external factors (like available bandwidth)
         // and internal factors (like number of parallel uploads).
         //
         // We use an exponential moving average here as a cheap way of smoothing
         // the chunk sizes a bit.
-        qint64 targetSize = propagator()->_chunkSize / 2 + predictedGoodSize / 2;
+        const auto targetSize = propagator()->_chunkSize / 2 + predictedGoodSize / 2;
 
         // Adjust the dynamic chunk size _chunkSize used for sizing of the item's chunks to be send
-        propagator()->_chunkSize = ::qBound(propagator()->syncOptions().minChunkSize(), targetSize, propagator()->syncOptions().maxChunkSize());
+        propagator()->_chunkSize = opts.toValidChunkSize(targetSize);
 
         qCInfo(lcPropagateUploadNG) << "Chunked upload of" << _currentChunkSize << "bytes took" << uploadTime.count()
-                                  << "ms, desired is" << targetDuration.count() << "ms, expected good chunk size is"
-                                  << predictedGoodSize << "bytes and nudged next chunk size to "
-                                  << propagator()->_chunkSize << "bytes";
+                                    << "ms, desired is" << opts._targetChunkUploadDuration.count() << "ms, expected"
+                                    << "good chunk size is" << predictedGoodSize << "bytes and nudged next chunk size"
+                                    << "to" << propagator()->_chunkSize << "bytes";
+
+        // Remembering the new chunk size in account so that it can be reused
+        const auto account = propagator()->account();
+        qCDebug(lcPropagateUploadNG) << "Reusing chunk size of" << propagator()->_chunkSize << "bytes"
+                                     << "in the next sync with" << account->displayName();
+        account->setLastChunkSize(propagator()->_chunkSize);
     }
 
     _finished = _sent == _item->_size;

diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp
@@ -215,6 +215,7 @@ void PropagateUploadFileV1::slotPutFinished()
     _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     _item->_responseTimeStamp = job->responseTimestamp();
     _item->_requestId = job->requestId();
+    _item->_requestBodySize = job->device()->size();
     QNetworkReply::NetworkError err = job->reply()->error();
     if (err != QNetworkReply::NoError) {
         commonErrorHandling(job);

diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h
@@ -287,6 +287,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem
     QString _errorString; // Contains a string only in case of error
     QByteArray _responseTimeStamp;
     QByteArray _requestId; // X-Request-Id of the failed request
+    qint64 _requestBodySize = -1; // the number of bytes sent. -1 if unknown.
     quint32 _affectedItems = 1; // the number of affected items by the operation on this item.
     // usually this value is 1, but for removes on dirs, it might be much higher.
 

diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp
@@ -13,6 +13,7 @@
  */
 
 #include "syncoptions.h"
+#include "account.h"
 #include "common/utility.h"
 
 #include <QRegularExpression>
@@ -31,34 +32,46 @@ qint64 SyncOptions::minChunkSize() const
     return _minChunkSize;
 }
 
-void SyncOptions::setMinChunkSize(const qint64 minChunkSize)
+void SyncOptions::setMinChunkSize(const qint64 value)
 {
-    _minChunkSize = ::qBound(_minChunkSize, minChunkSize, _maxChunkSize);
+    _minChunkSize = qBound(chunkV2MinChunkSize, value, _maxChunkSize);
+    setInitialChunkSize(_initialChunkSize);
 }
 
 qint64 SyncOptions::maxChunkSize() const
 {
     return _maxChunkSize;
 }
 
-void SyncOptions::setMaxChunkSize(const qint64 maxChunkSize)
+void SyncOptions::setMaxChunkSize(const qint64 value)
 {
-    _maxChunkSize = ::qBound(_minChunkSize, maxChunkSize, _maxChunkSize);
+    _maxChunkSize = qBound(_minChunkSize, value, chunkV2MaxChunkSize);
+    setInitialChunkSize(_initialChunkSize);
 }
 
-void SyncOptions::fillFromEnvironmentVariables()
+qint64 SyncOptions::initialChunkSize() const
 {
-    QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE");
-    if (!chunkSizeEnv.isEmpty())
-        _initialChunkSize = chunkSizeEnv.toUInt();
+    return _initialChunkSize;
+}
 
+void SyncOptions::setInitialChunkSize(const qint64 value)
+{
+    _initialChunkSize = toValidChunkSize(value);
+}
+
+void SyncOptions::fillFromEnvironmentVariables()
+{
     QByteArray minChunkSizeEnv = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE");
     if (!minChunkSizeEnv.isEmpty())
-        _minChunkSize = minChunkSizeEnv.toUInt();
+        setMinChunkSize(minChunkSizeEnv.toUInt());
 
     QByteArray maxChunkSizeEnv = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE");
     if (!maxChunkSizeEnv.isEmpty())
-        _maxChunkSize = maxChunkSizeEnv.toUInt();
+        setMaxChunkSize(maxChunkSizeEnv.toUInt());
+
+    QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE");
+    if (!chunkSizeEnv.isEmpty())
+        setInitialChunkSize(chunkSizeEnv.toUInt());
 
     QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION");
     if (!targetChunkUploadDurationEnv.isEmpty())
@@ -69,10 +82,35 @@ void SyncOptions::fillFromEnvironmentVariables()
         _parallelNetworkJobs = maxParallel;
 }
 
-void SyncOptions::verifyChunkSizes()
+void SyncOptions::fillFromAccount(const AccountPtr account)
+{
+    if (account) {
+        if (account->isHttp2Supported() && _parallelNetworkJobs == defaultParallelNetworkJobs) {
+            _parallelNetworkJobs = defaultParallelNetworkJobsH2;
+        }
+
+        if (const auto size = account->getMaxRequestSize(); size > 0) {
+            setMaxChunkSize(size);
+        }
+
+        if (account->capabilities().chunkingNg()) {
+            // read last successfully used chunk size and use it as initial value
+            if (const auto size = account->getLastChunkSize(); size > 0) {
+                setInitialChunkSize(size);
+            }
+        } else {
+            // disable dynamic chunk sizing as it is not supported for this account
+            _targetChunkUploadDuration = std::chrono::milliseconds(0);
+        }
+    }
+}
+
+qint64 SyncOptions::predictedGoodChunkSize(const qint64 currentChunkSize, const std::chrono::milliseconds uploadTime) const
 {
-    _minChunkSize = qMin(_minChunkSize, _initialChunkSize);
-    _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize);
+    if (isDynamicChunkSize() && uploadTime.count() > 0) {
+        return (currentChunkSize * _targetChunkUploadDuration) / uploadTime;
+    }
+    return currentChunkSize;
 }
 
 QRegularExpression SyncOptions::fileRegex() const