Skip to content

Commit

Permalink
Upload: Auto limit max chunk size on HTTP-413
Browse files Browse the repository at this point in the history
 * 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]>
  • Loading branch information
jkellerer committed Sep 22, 2023
1 parent 395edd9 commit 493b526
Show file tree
Hide file tree
Showing 18 changed files with 427 additions and 96 deletions.
2 changes: 1 addition & 1 deletion src/cmd/cmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/socketapi/socketuploadjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
21 changes: 21 additions & 0 deletions src/libsync/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#ifndef SERVERCONNECTION_H
#define SERVERCONNECTION_H

#include <QAtomicInteger>
#include <QByteArray>
#include <QUrl>
#include <QNetworkCookie>
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions src/libsync/configfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "config.h"

#include "configfile.h"
#include "syncoptions.h"
#include "theme.h"
#include "version.h"
#include "common/utility.h"
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 too 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
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/propagateupload.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 15 additions & 9 deletions src/libsync/propagateuploadng.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/libsync/propagateuploadv1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/libsync/syncfileitem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
66 changes: 53 additions & 13 deletions src/libsync/syncoptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/

#include "syncoptions.h"
#include "account.h"
#include "common/utility.h"

#include <QRegularExpression>
Expand All @@ -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())
Expand All @@ -69,10 +82,37 @@ void SyncOptions::fillFromEnvironmentVariables()
_parallelNetworkJobs = maxParallel;
}

void SyncOptions::verifyChunkSizes()
void SyncOptions::fillFromAccount(const AccountPtr account)
{
if (!account) {
return;
}

if (account->isHttp2Supported() && _parallelNetworkJobs == defaultParallelNetworkJobs) {
_parallelNetworkJobs = defaultParallelNetworkJobsH2;
}

if (const auto size = account->getMaxRequestSize(); size > 0) {
setMaxChunkSize(size);
}

if (account->capabilities().chunkingNg()) {
// read last 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
Expand Down
Loading

0 comments on commit 493b526

Please sign in to comment.