Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload: Auto adjust maxChunkSize until upload succeeds after HTTP-413 #4826

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1098,16 +1099,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
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/account.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/account.h

File src/libsync/account.h does not conform to Custom style guidelines. (lines 19, 282, 283, 284, 296, 297)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -16,6 +16,7 @@
#ifndef SERVERCONNECTION_H
#define SERVERCONNECTION_H

#include <QAtomicInteger>

Check failure on line 19 in src/libsync/account.h

View workflow job for this annotation

GitHub Actions / build

/src/libsync/account.h:19:10 [clang-diagnostic-error]

'QAtomicInteger' file not found
#include <QByteArray>
#include <QUrl>
#include <QNetworkCookie>
Expand Down Expand Up @@ -277,6 +278,24 @@
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 @@ -426,6 +445,8 @@
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 @@ -433,8 +433,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 @@ -744,7 +744,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
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/propagateupload.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/propagateupload.cpp

File src/libsync/propagateupload.cpp does not conform to Custom style guidelines. (lines 695, 699, 704, 707, 708, 709, 710, 711, 718, 726, 727)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -686,6 +686,49 @@
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
jkellerer marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/propagateuploadng.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/propagateuploadng.cpp

File src/libsync/propagateuploadng.cpp does not conform to Custom style guidelines. (lines 443)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -410,6 +410,7 @@
if (err != QNetworkReply::NoError) {
_item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
_item->_requestId = job->requestId();
_item->_requestBodySize = job->device()->size();
commonErrorHandling(job);
const auto exceptionParsed = getExceptionFromReply(job->reply());
_item->_errorExceptionName = exceptionParsed.first;
Expand All @@ -423,26 +424,31 @@
//
// 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 @@ -291,6 +291,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem
QString _errorExceptionMessage; // Contains a server exception message 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
Loading