From 88e0f97798957da1ba4a789e39819a061ac9dda3 Mon Sep 17 00:00:00 2001 From: Piotr M Date: Wed, 7 Dec 2016 22:35:50 +0100 Subject: [PATCH 1/5] dynamic chunking prototype --- src/libsync/capabilities.cpp | 8 +++++++ src/libsync/capabilities.h | 1 + src/libsync/configfile.cpp | 13 ++++++++++ src/libsync/configfile.h | 2 ++ src/libsync/owncloudpropagator.cpp | 14 ++++++++++- src/libsync/owncloudpropagator.h | 2 +- src/libsync/propagateupload.h | 29 +++++++++++++++++++++-- src/libsync/propagateuploadng.cpp | 38 ++++++++++++++++++++++++++++-- 8 files changed, 101 insertions(+), 6 deletions(-) diff --git a/src/libsync/capabilities.cpp b/src/libsync/capabilities.cpp index cee5f1aebe2..0c57eab1eae 100644 --- a/src/libsync/capabilities.cpp +++ b/src/libsync/capabilities.cpp @@ -130,4 +130,12 @@ QList Capabilities::httpErrorCodesThatResetFailingChunkedUploads() const return list; } +quint64 Capabilities::requestMaxDurationDC() const +{ + QByteArray requestMaxDurationDC = _capabilities["dav"].toMap()["max_single_upload_request_duration_msec"].toByteArray(); + if (!requestMaxDurationDC.isEmpty()) + return requestMaxDurationDC.toLongLong(); + return 0; +} + } diff --git a/src/libsync/capabilities.h b/src/libsync/capabilities.h index e3d8c013c23..0c28efa6145 100644 --- a/src/libsync/capabilities.h +++ b/src/libsync/capabilities.h @@ -41,6 +41,7 @@ class OWNCLOUDSYNC_EXPORT Capabilities { int sharePublicLinkExpireDateDays() const; bool shareResharing() const; bool chunkingNg() const; + quint64 requestMaxDurationDC() const; /// disable parallel upload in chunking bool chunkingParallelUploadDisabled() const; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 2132ced728a..bad02f49ddd 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -53,6 +53,7 @@ static const char updateCheckIntervalC[] = "updateCheckInterval"; static const char geometryC[] = "geometry"; static const char timeoutC[] = "timeout"; static const char chunkSizeC[] = "chunkSize"; +static const char maxChunkSizeC[] = "maxChunkSizeC"; static const char proxyHostC[] = "Proxy/host"; static const char proxyTypeC[] = "Proxy/type"; @@ -130,6 +131,18 @@ quint64 ConfigFile::chunkSize() const return settings.value(QLatin1String(chunkSizeC), 10*1000*1000).toLongLong(); // default to 10 MB } +quint64 ConfigFile::maxChunkSize() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(QLatin1String(maxChunkSizeC), 50*1000*1000).toLongLong(); // default to 50 MB +} + +quint64 ConfigFile::minChunkSize() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(QLatin1String(maxChunkSizeC), 1000*1000).toLongLong(); // default to 1 MB +} + void ConfigFile::setOptionalDesktopNotifications(bool show) { QSettings settings(configFile(), QSettings::IniFormat); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 0fe3b3cd063..8191ab13e6e 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -115,6 +115,8 @@ class OWNCLOUDSYNC_EXPORT ConfigFile int timeout() const; quint64 chunkSize() const; + quint64 maxChunkSize() const; + quint64 minChunkSize() const; void saveGeometry(QWidget *w); void restoreGeometry(QWidget *w); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 300b7778af4..3c79a87f3e5 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -370,7 +370,7 @@ PropagateItemJob* OwncloudPropagator::createJob(const SyncFileItemPtr &item) { } else { PropagateUploadFileCommon *job = 0; if (item->_size > chunkSize() && account()->capabilities().chunkingNg()) { - job = new PropagateUploadFileNG(this, item); + job = new PropagateUploadFileNG(this, item, account()->capabilities().requestMaxDurationDC()); } else { job = new PropagateUploadFileV1(this, item); } @@ -547,6 +547,18 @@ quint64 OwncloudPropagator::chunkSize() return chunkSize; } +quint64 OwncloudPropagator::maxChunkSize() +{ + static uint chunkSize; + if (!chunkSize) { + chunkSize = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE").toUInt(); + if (chunkSize == 0) { + ConfigFile cfg; + chunkSize = cfg.maxChunkSize(); + } + } + return chunkSize; +} bool OwncloudPropagator::localFileNameClash( const QString& relFile ) { diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 0cb949e39b8..be913b75386 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -279,7 +279,6 @@ class OwncloudPropagator : public QObject { SyncJournalDb * const _journal; bool _finishedEmited; // used to ensure that finished is only emitted once - public: OwncloudPropagator(AccountPtr account, const QString &localDir, const QString &remoteFolder, SyncJournalDb *progressDb) @@ -357,6 +356,7 @@ class OwncloudPropagator : public QObject { /** returns the size of chunks in bytes */ static quint64 chunkSize(); + static quint64 maxChunkSize(); AccountPtr account() const; diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index 3a7224a7019..cf6e5b7956f 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -304,6 +304,24 @@ class PropagateUploadFileNG : public PropagateUploadFileCommon { uint _transferId; /// transfer id (part of the url) int _currentChunk; /// Id of the next chunk that will be sent bool _removeJobError; /// If not null, there was an error removing the job + quint64 _lastChunkSize; /// current chunk size + + /* + * This is value in ms obtained from the server. + * + * Dynamic Chunking attribute the maximum number of miliseconds that single request below chunk size can take + * This value should be based on heuristics with default value 10000ms, time it takes to transfer 10MB chunk on 1MB/s upload link. + * + * Suggested solution will be to evaluate max(SNR, MORD) where: + * > SNR - Slow network request, so time it will take to transmit default chunking sized request at specific low upload bandwidth + * > MORD - Maximum observed request time, so double the time of maximum observed RTT of the very small PUT request (e.g. 1kB) to the system + * + * Exemplary, syncing 100MB files, with chunking size 10MB, will cause sync of 10 PUT requests which max evaluation was set to + * + * Dynamic chunking client algorithm is specified in the ownCloud documentation and uses to estimate if given + * bandwidth allows higher chunk sizes (because of high goodput) + */ + quint64 _requestMaxDuration; // Map chunk number with its size from the PROPFIND on resume. // (Only used from slotPropfindIterate/slotPropfindFinished because the LsColJob use signals to report data.) @@ -311,6 +329,12 @@ class PropagateUploadFileNG : public PropagateUploadFileCommon { QMap _serverChunks; quint64 chunkSize() const { return propagator()->chunkSize(); } + quint64 maxChunkSize() const { return propagator()->maxChunkSize(); } + + quint64 getRequestMaxDurationDC(){ + return _requestMaxDuration; + } + /** * Return the URL of a chunk. * If chunk == -1, returns the URL of the parent folder containing the chunks @@ -318,10 +342,11 @@ class PropagateUploadFileNG : public PropagateUploadFileCommon { QUrl chunkUrl(int chunk = -1); public: - PropagateUploadFileNG(OwncloudPropagator* propagator,const SyncFileItemPtr& item) : - PropagateUploadFileCommon(propagator,item) {} + PropagateUploadFileNG(OwncloudPropagator* propagator,const SyncFileItemPtr& item, const quint64& requestMaxDuration) : + PropagateUploadFileCommon(propagator,item), _lastChunkSize(0), _requestMaxDuration(requestMaxDuration) {} void doStartUpload() Q_DECL_OVERRIDE; + private: void startNewUpload(); void startNextChunk(); diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 9803385be10..63d71d3c4d9 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -32,7 +32,7 @@ #include #include #include - +#include namespace OCC { QUrl PropagateUploadFileNG::chunkUrl(int chunk) @@ -272,7 +272,41 @@ void PropagateUploadFileNG::startNextChunk() quint64 fileSize = _item->_size; ENFORCE(fileSize >= _sent, "Sent data exceeds file size"); - quint64 currentChunkSize = qMin(chunkSize(), fileSize - _sent); + quint64 currentChunkSize = chunkSize(); + + // this will check if getRequestMaxDurationDC is set to 0 or not + double requestMaxDurationDC = (double) getRequestMaxDurationDC(); + if (requestMaxDurationDC != 0) { + // this if first chunked file request, so it can start with default size of chunkSize() + // if _lastChunkSize != 0 it means that we already have send one request + if(_lastChunkSize != 0){ + //TODO: this is done step by step for debugging purposes + + //get last request timestamp + double lastChunkLap = (double) _stopWatch.durationOfLap(QLatin1String("ChunkDuration")); + + //get duration of the request + double requestDuration = (double) _stopWatch.addLapTime(QLatin1String("ChunkDuration")) - lastChunkLap; + + // calculate natural logarithm + double correctionParameter = log(requestMaxDurationDC / requestDuration) - 1; + + // If logarithm is smaller or equal zero, it means that we exceeded max request duration + // If exceeded it will use currentChunkSize = chunkSize() + // If did not exceeded, we will increase the chunk size + // motivation for logarithm is specified in the dynamic chunking documentation + // TODO: give link to documentation + if (correctionParameter>0){ + currentChunkSize = qMin(_lastChunkSize + (qint64) correctionParameter*chunkSize(), maxChunkSize()); + } + } + + //remember the value of last chunk size + _lastChunkSize = currentChunkSize; + } + + // prevent situation that chunk size is bigger then required one to send + currentChunkSize = qMin(currentChunkSize, fileSize - _sent); if (currentChunkSize == 0) { ASSERT(_jobs.isEmpty()); From 0468f44f5ce676d834a385b37756f475e18aa8a2 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 24 Mar 2017 15:01:50 +0100 Subject: [PATCH 2/5] Various fixes and improvements --- src/libsync/capabilities.cpp | 9 ++-- src/libsync/capabilities.h | 12 ++++- src/libsync/configfile.cpp | 3 +- src/libsync/owncloudpropagator.cpp | 25 ++++++++--- src/libsync/owncloudpropagator.h | 13 +++++- src/libsync/propagateupload.cpp | 1 + src/libsync/propagateupload.h | 39 +++++------------ src/libsync/propagateuploadng.cpp | 70 +++++++++++++----------------- 8 files changed, 91 insertions(+), 81 deletions(-) diff --git a/src/libsync/capabilities.cpp b/src/libsync/capabilities.cpp index 0c57eab1eae..213a36d789f 100644 --- a/src/libsync/capabilities.cpp +++ b/src/libsync/capabilities.cpp @@ -130,11 +130,12 @@ QList Capabilities::httpErrorCodesThatResetFailingChunkedUploads() const return list; } -quint64 Capabilities::requestMaxDurationDC() const +quint64 Capabilities::desiredChunkUploadDuration() const { - QByteArray requestMaxDurationDC = _capabilities["dav"].toMap()["max_single_upload_request_duration_msec"].toByteArray(); - if (!requestMaxDurationDC.isEmpty()) - return requestMaxDurationDC.toLongLong(); + QByteArray value = _capabilities["dav"].toMap()["target_chunk_upload_request_duration_msec"].toByteArray(); + if (!value.isEmpty()) { + return value.toLongLong(); + } return 0; } diff --git a/src/libsync/capabilities.h b/src/libsync/capabilities.h index 0c28efa6145..9a3270d013a 100644 --- a/src/libsync/capabilities.h +++ b/src/libsync/capabilities.h @@ -41,7 +41,17 @@ class OWNCLOUDSYNC_EXPORT Capabilities { int sharePublicLinkExpireDateDays() const; bool shareResharing() const; bool chunkingNg() const; - quint64 requestMaxDurationDC() const; + + /** + * The desired time in ms needed for a single-chunk upload. + * + * The chunk size will be dynamically adjusted to target + * this value. + * + * Capability: dav/target_chunk_upload_request_duration_msec + */ + quint64 desiredChunkUploadDuration() const; + /// disable parallel upload in chunking bool chunkingParallelUploadDisabled() const; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index bad02f49ddd..11529f2e06a 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -53,6 +53,7 @@ static const char updateCheckIntervalC[] = "updateCheckInterval"; static const char geometryC[] = "geometry"; static const char timeoutC[] = "timeout"; static const char chunkSizeC[] = "chunkSize"; +static const char minChunkSizeC[] = "minChunkSizeC"; static const char maxChunkSizeC[] = "maxChunkSizeC"; static const char proxyHostC[] = "Proxy/host"; @@ -140,7 +141,7 @@ quint64 ConfigFile::maxChunkSize() const quint64 ConfigFile::minChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(maxChunkSizeC), 1000*1000).toLongLong(); // default to 1 MB + return settings.value(QLatin1String(minChunkSizeC), 1000*1000).toLongLong(); // default to 1 MB } void ConfigFile::setOptionalDesktopNotifications(bool show) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 3c79a87f3e5..35844d84bad 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -369,8 +369,8 @@ PropagateItemJob* OwncloudPropagator::createJob(const SyncFileItemPtr &item) { return job; } else { PropagateUploadFileCommon *job = 0; - if (item->_size > chunkSize() && account()->capabilities().chunkingNg()) { - job = new PropagateUploadFileNG(this, item, account()->capabilities().requestMaxDurationDC()); + if (item->_size > _chunkSize && account()->capabilities().chunkingNg()) { + job = new PropagateUploadFileNG(this, item); } else { job = new PropagateUploadFileV1(this, item); } @@ -522,7 +522,7 @@ bool OwncloudPropagator::isInSharedDirectory(const QString& file) int OwncloudPropagator::httpTimeout() { - static int timeout; + static int timeout = 0; if (!timeout) { timeout = qgetenv("OWNCLOUD_TIMEOUT").toUInt(); if (timeout == 0) { @@ -534,9 +534,9 @@ int OwncloudPropagator::httpTimeout() return timeout; } -quint64 OwncloudPropagator::chunkSize() +quint64 OwncloudPropagator::initialChunkSize() { - static uint chunkSize; + static uint chunkSize = 0; if (!chunkSize) { chunkSize = qgetenv("OWNCLOUD_CHUNK_SIZE").toUInt(); if (chunkSize == 0) { @@ -549,7 +549,7 @@ quint64 OwncloudPropagator::chunkSize() quint64 OwncloudPropagator::maxChunkSize() { - static uint chunkSize; + static uint chunkSize = 0; if (!chunkSize) { chunkSize = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE").toUInt(); if (chunkSize == 0) { @@ -560,6 +560,19 @@ quint64 OwncloudPropagator::maxChunkSize() return chunkSize; } +quint64 OwncloudPropagator::minChunkSize() +{ + static uint chunkSize = 0; + if (!chunkSize) { + chunkSize = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE").toUInt(); + if (chunkSize == 0) { + ConfigFile cfg; + chunkSize = cfg.minChunkSize(); + } + } + return chunkSize; +} + bool OwncloudPropagator::localFileNameClash( const QString& relFile ) { bool re = false; diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index be913b75386..4d632fde0d8 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -288,6 +288,7 @@ class OwncloudPropagator : public QObject { , _finishedEmited(false) , _bandwidthManager(this) , _anotherSyncNeeded(false) + , _chunkSize(initialChunkSize()) , _account(account) { } @@ -314,6 +315,15 @@ class OwncloudPropagator : public QObject { /* the maximum number of jobs using bandwidth (uploads or downloads, in parallel) */ int maximumActiveTransferJob(); + + /** The size to use for upload chunks. + * + * Will be dynamically adjusted after each chunk upload finishes + * if Capabilities::desiredChunkUploadDuration has a target + * chunk-upload duration set. + */ + quint64 _chunkSize; + /* The maximum number of active jobs in parallel */ int hardMaximumActiveJob(); @@ -355,8 +365,9 @@ class OwncloudPropagator : public QObject { static int httpTimeout(); /** returns the size of chunks in bytes */ - static quint64 chunkSize(); + static quint64 initialChunkSize(); static quint64 maxChunkSize(); + static quint64 minChunkSize(); AccountPtr account() const; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 56a46d4e4f2..62dd386fd1e 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -97,6 +97,7 @@ void PUTFileJob::start() { connect(_device, SIGNAL(wasReset()), this, SLOT(slotSoftAbort())); #endif + _requestTimer.start(); AbstractNetworkJob::start(); } diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index cf6e5b7956f..69f095f767f 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -19,6 +19,7 @@ #include #include #include +#include namespace OCC { @@ -90,6 +91,7 @@ class PUTFileJob : public AbstractNetworkJob { QMap _headers; QString _errorString; QUrl _url; + QElapsedTimer _requestTimer; public: // Takes ownership of the device @@ -123,6 +125,10 @@ class PUTFileJob : public AbstractNetworkJob { virtual void slotTimeout() Q_DECL_OVERRIDE; + quint64 msSinceStart() const { + return _requestTimer.elapsed(); + } + signals: void finishedSignal(); @@ -201,7 +207,6 @@ class PropagateUploadFileCommon : public PropagateItemJob { QByteArray _transmissionChecksum; QByteArray _transmissionChecksumType; - public: PropagateUploadFileCommon(OwncloudPropagator* propagator,const SyncFileItemPtr& item) : PropagateItemJob(propagator, item), _finished(false), _deleteExisting(false) {} @@ -276,7 +281,7 @@ class PropagateUploadFileV1 : public PropagateUploadFileCommon { int _chunkCount; /// Total number of chunks for this file int _transferId; /// transfer id (part of the url) - quint64 chunkSize() const { return propagator()->chunkSize(); } + quint64 chunkSize() const { return propagator()->initialChunkSize(); } public: @@ -303,38 +308,14 @@ class PropagateUploadFileNG : public PropagateUploadFileCommon { quint64 _sent; /// amount of data (bytes) that was already sent uint _transferId; /// transfer id (part of the url) int _currentChunk; /// Id of the next chunk that will be sent + quint64 _currentChunkSize; /// current chunk size bool _removeJobError; /// If not null, there was an error removing the job - quint64 _lastChunkSize; /// current chunk size - - /* - * This is value in ms obtained from the server. - * - * Dynamic Chunking attribute the maximum number of miliseconds that single request below chunk size can take - * This value should be based on heuristics with default value 10000ms, time it takes to transfer 10MB chunk on 1MB/s upload link. - * - * Suggested solution will be to evaluate max(SNR, MORD) where: - * > SNR - Slow network request, so time it will take to transmit default chunking sized request at specific low upload bandwidth - * > MORD - Maximum observed request time, so double the time of maximum observed RTT of the very small PUT request (e.g. 1kB) to the system - * - * Exemplary, syncing 100MB files, with chunking size 10MB, will cause sync of 10 PUT requests which max evaluation was set to - * - * Dynamic chunking client algorithm is specified in the ownCloud documentation and uses to estimate if given - * bandwidth allows higher chunk sizes (because of high goodput) - */ - quint64 _requestMaxDuration; // Map chunk number with its size from the PROPFIND on resume. // (Only used from slotPropfindIterate/slotPropfindFinished because the LsColJob use signals to report data.) struct ServerChunkInfo { quint64 size; QString originalName; }; QMap _serverChunks; - quint64 chunkSize() const { return propagator()->chunkSize(); } - quint64 maxChunkSize() const { return propagator()->maxChunkSize(); } - - quint64 getRequestMaxDurationDC(){ - return _requestMaxDuration; - } - /** * Return the URL of a chunk. * If chunk == -1, returns the URL of the parent folder containing the chunks @@ -342,8 +323,8 @@ class PropagateUploadFileNG : public PropagateUploadFileCommon { QUrl chunkUrl(int chunk = -1); public: - PropagateUploadFileNG(OwncloudPropagator* propagator,const SyncFileItemPtr& item, const quint64& requestMaxDuration) : - PropagateUploadFileCommon(propagator,item), _lastChunkSize(0), _requestMaxDuration(requestMaxDuration) {} + PropagateUploadFileNG(OwncloudPropagator* propagator,const SyncFileItemPtr& item) : + PropagateUploadFileCommon(propagator,item), _currentChunkSize(0) {} void doStartUpload() Q_DECL_OVERRIDE; diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 63d71d3c4d9..a4589cef8e6 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -32,7 +32,7 @@ #include #include #include -#include + namespace OCC { QUrl PropagateUploadFileNG::chunkUrl(int chunk) @@ -272,44 +272,11 @@ void PropagateUploadFileNG::startNextChunk() quint64 fileSize = _item->_size; ENFORCE(fileSize >= _sent, "Sent data exceeds file size"); - quint64 currentChunkSize = chunkSize(); - - // this will check if getRequestMaxDurationDC is set to 0 or not - double requestMaxDurationDC = (double) getRequestMaxDurationDC(); - if (requestMaxDurationDC != 0) { - // this if first chunked file request, so it can start with default size of chunkSize() - // if _lastChunkSize != 0 it means that we already have send one request - if(_lastChunkSize != 0){ - //TODO: this is done step by step for debugging purposes - - //get last request timestamp - double lastChunkLap = (double) _stopWatch.durationOfLap(QLatin1String("ChunkDuration")); - - //get duration of the request - double requestDuration = (double) _stopWatch.addLapTime(QLatin1String("ChunkDuration")) - lastChunkLap; - - // calculate natural logarithm - double correctionParameter = log(requestMaxDurationDC / requestDuration) - 1; - - // If logarithm is smaller or equal zero, it means that we exceeded max request duration - // If exceeded it will use currentChunkSize = chunkSize() - // If did not exceeded, we will increase the chunk size - // motivation for logarithm is specified in the dynamic chunking documentation - // TODO: give link to documentation - if (correctionParameter>0){ - currentChunkSize = qMin(_lastChunkSize + (qint64) correctionParameter*chunkSize(), maxChunkSize()); - } - } - - //remember the value of last chunk size - _lastChunkSize = currentChunkSize; - } - // prevent situation that chunk size is bigger then required one to send - currentChunkSize = qMin(currentChunkSize, fileSize - _sent); + _currentChunkSize = qMin(propagator()->_chunkSize, fileSize - _sent); - if (currentChunkSize == 0) { - ASSERT(_jobs.isEmpty()); + if (_currentChunkSize == 0) { + Q_ASSERT(_jobs.isEmpty()); // There should be no running job anymore _finished = true; // Finish with a MOVE QString destination = QDir::cleanPath(propagator()->account()->url().path() + QLatin1Char('/') @@ -339,7 +306,7 @@ void PropagateUploadFileNG::startNextChunk() auto device = new UploadDevice(&propagator()->_bandwidthManager); const QString fileName = propagator()->getFilePath(_item->_file); - if (! device->prepareAndOpen(fileName, _sent, currentChunkSize)) { + if (! device->prepareAndOpen(fileName, _sent, _currentChunkSize)) { qDebug() << "ERR: Could not prepare upload device: " << device->errorString(); // If the file is currently locked, we want to retry the sync @@ -355,7 +322,7 @@ void PropagateUploadFileNG::startNextChunk() QMap headers; headers["OC-Chunk-Offset"] = QByteArray::number(_sent); - _sent += currentChunkSize; + _sent += _currentChunkSize; QUrl url = chunkUrl(_currentChunk); // job takes ownership of device via a QScopedPointer. Job deletes itself when finishing @@ -428,6 +395,31 @@ void PropagateUploadFileNG::slotPutFinished() } ENFORCE(_sent <= _item->_size, "can't send more than size"); + + // Adjust the chunk size for the time taken. + // + // Dynamic chunk sizing is enabled if the server configured a + // target duration for each chunk upload. + double targetDuration = propagator()->account()->capabilities().desiredChunkUploadDuration(); + if (targetDuration > 0) { + double uploadTime = job->msSinceStart(); + + auto correctedSize = static_cast( + _currentChunkSize / uploadTime * targetDuration); + + // There can be multiple chunk uploads going on at the same time. + // So don't force the chunk size to the new predicted best size + // and instead move it there gradually. + propagator()->_chunkSize = qBound( + propagator()->minChunkSize(), + (propagator()->_chunkSize + correctedSize) / 2, + propagator()->maxChunkSize()); + + qDebug() << "Chunked upload of " << _currentChunkSize << " took " << uploadTime + << " desired is " << targetDuration << ", expected good chunk size is " + << correctedSize << " and nudged next chunk size to " << propagator()->_chunkSize; + } + bool finished = _sent == _item->_size; // Check if the file still exists From 6aad228566852f9ea3c4122401563bcd1f96d709 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 27 Mar 2017 14:12:13 +0200 Subject: [PATCH 3/5] Put min/max chunk sizes into SyncOptions --- src/gui/folder.cpp | 23 ++++++++++++++ src/libsync/discoveryphase.h | 24 +++++++++++++- src/libsync/owncloudpropagator.cpp | 50 +++++++----------------------- src/libsync/owncloudpropagator.h | 12 +++---- src/libsync/propagateupload.h | 2 +- src/libsync/propagateuploadng.cpp | 4 +-- src/libsync/syncengine.cpp | 1 + 7 files changed, 67 insertions(+), 49 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 18d2f2b12f7..4b938157ef0 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -648,9 +648,32 @@ void Folder::startSync(const QStringList &pathList) SyncOptions opt; ConfigFile cfgFile; + auto newFolderLimit = cfgFile.newBigFolderSizeLimit(); opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); + + opt._initialChunkSize = qgetenv("OWNCLOUD_CHUNK_SIZE").toUInt(); + if (opt._initialChunkSize == 0) { + opt._initialChunkSize = cfgFile.chunkSize(); + } + opt._minChunkSize = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE").toUInt(); + if (opt._minChunkSize == 0) { + opt._minChunkSize = cfgFile.minChunkSize(); + } + opt._maxChunkSize = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE").toUInt(); + if (opt._maxChunkSize == 0) { + opt._maxChunkSize = cfgFile.maxChunkSize(); + } + + // Previously min/max chunk size values didn't exist, so users might + // have setups where the chunk size exceeds the new min/max default + // values. To cope with this, adjust min/max to always include the + // initial chunk size value. + opt._minChunkSize = qMin(opt._minChunkSize, opt._initialChunkSize); + opt._maxChunkSize = qMax(opt._maxChunkSize, opt._initialChunkSize); + + _engine->setSyncOptions(opt); _engine->setIgnoreHiddenFiles(_definition.ignoreHiddenFiles); diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 3bccf6a4a13..49c42eb2818 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -35,12 +35,34 @@ class Account; */ struct SyncOptions { - SyncOptions() : _newBigFolderSizeLimit(-1), _confirmExternalStorage(false) {} + SyncOptions() + : _newBigFolderSizeLimit(-1) + , _confirmExternalStorage(false) + , _initialChunkSize(10 * 1000 * 1000) // 10 MB + , _minChunkSize(1 * 1000 * 1000) // 1 MB + , _maxChunkSize(100 * 1000 * 1000) // 100 MB + {} + /** Maximum size (in Bytes) a folder can have without asking for confirmation. * -1 means infinite */ qint64 _newBigFolderSizeLimit; + /** If a confirmation should be asked for external storages */ bool _confirmExternalStorage; + + /** The initial un-adjusted chunk size in bytes for chunked uploads + * + * When dynamic chunk size adjustments are done, this is the + * starting value and is then gradually adjusted within the + * minChunkSize / maxChunkSize bounds. + */ + quint64 _initialChunkSize; + + /** The minimum chunk size in bytes for chunked uploads */ + quint64 _minChunkSize; + + /** The maximum chunk size in bytes for chunked uploads */ + quint64 _maxChunkSize; }; diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 35844d84bad..232984e0488 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -503,6 +503,17 @@ void OwncloudPropagator::start(const SyncFileItemVector& items) scheduleNextJob(); } +const SyncOptions& OwncloudPropagator::syncOptions() const +{ + return _syncOptions; +} + +void OwncloudPropagator::setSyncOptions(const SyncOptions& syncOptions) +{ + _syncOptions = syncOptions; + _chunkSize = syncOptions._initialChunkSize; +} + // ownCloud server < 7.0 did not had permissions so we need some other euristics // to detect wrong doing in a Shared directory bool OwncloudPropagator::isInSharedDirectory(const QString& file) @@ -534,45 +545,6 @@ int OwncloudPropagator::httpTimeout() return timeout; } -quint64 OwncloudPropagator::initialChunkSize() -{ - static uint chunkSize = 0; - if (!chunkSize) { - chunkSize = qgetenv("OWNCLOUD_CHUNK_SIZE").toUInt(); - if (chunkSize == 0) { - ConfigFile cfg; - chunkSize = cfg.chunkSize(); - } - } - return chunkSize; -} - -quint64 OwncloudPropagator::maxChunkSize() -{ - static uint chunkSize = 0; - if (!chunkSize) { - chunkSize = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE").toUInt(); - if (chunkSize == 0) { - ConfigFile cfg; - chunkSize = cfg.maxChunkSize(); - } - } - return chunkSize; -} - -quint64 OwncloudPropagator::minChunkSize() -{ - static uint chunkSize = 0; - if (!chunkSize) { - chunkSize = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE").toUInt(); - if (chunkSize == 0) { - ConfigFile cfg; - chunkSize = cfg.minChunkSize(); - } - } - return chunkSize; -} - bool OwncloudPropagator::localFileNameClash( const QString& relFile ) { bool re = false; diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 4d632fde0d8..b2a57f7da0a 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -29,6 +29,7 @@ #include "syncjournaldb.h" #include "bandwidthmanager.h" #include "accountfwd.h" +#include "discoveryphase.h" namespace OCC { @@ -288,7 +289,7 @@ class OwncloudPropagator : public QObject { , _finishedEmited(false) , _bandwidthManager(this) , _anotherSyncNeeded(false) - , _chunkSize(initialChunkSize()) + , _chunkSize(10 * 1000 * 1000) // 10 MB, overridden in setSyncOptions , _account(account) { } @@ -296,6 +297,9 @@ class OwncloudPropagator : public QObject { void start(const SyncFileItemVector &_syncedItems); + const SyncOptions& syncOptions() const; + void setSyncOptions(const SyncOptions& syncOptions); + QAtomicInt _downloadLimit; QAtomicInt _uploadLimit; BandwidthManager _bandwidthManager; @@ -364,11 +368,6 @@ class OwncloudPropagator : public QObject { // timeout in seconds static int httpTimeout(); - /** returns the size of chunks in bytes */ - static quint64 initialChunkSize(); - static quint64 maxChunkSize(); - static quint64 minChunkSize(); - AccountPtr account() const; enum DiskSpaceResult @@ -415,6 +414,7 @@ private slots: AccountPtr _account; QScopedPointer _rootJob; + SyncOptions _syncOptions; #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) // access to signals which are protected in Qt4 diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index 69f095f767f..c031602bc90 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -281,7 +281,7 @@ class PropagateUploadFileV1 : public PropagateUploadFileCommon { int _chunkCount; /// Total number of chunks for this file int _transferId; /// transfer id (part of the url) - quint64 chunkSize() const { return propagator()->initialChunkSize(); } + quint64 chunkSize() const { return propagator()->syncOptions()._initialChunkSize; } public: diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index a4589cef8e6..733b9eb7d09 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -411,9 +411,9 @@ void PropagateUploadFileNG::slotPutFinished() // So don't force the chunk size to the new predicted best size // and instead move it there gradually. propagator()->_chunkSize = qBound( - propagator()->minChunkSize(), + propagator()->syncOptions()._minChunkSize, (propagator()->_chunkSize + correctedSize) / 2, - propagator()->maxChunkSize()); + propagator()->syncOptions()._maxChunkSize); qDebug() << "Chunked upload of " << _currentChunkSize << " took " << uploadTime << " desired is " << targetDuration << ", expected good chunk size is " diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 5aa2376b0a5..623734a8353 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -1004,6 +1004,7 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult) _propagator = QSharedPointer( new OwncloudPropagator (_account, _localPath, _remotePath, _journal)); + _propagator->setSyncOptions(_syncOptions); connect(_propagator.data(), SIGNAL(itemCompleted(const SyncFileItemPtr &)), this, SLOT(slotItemCompleted(const SyncFileItemPtr &))); connect(_propagator.data(), SIGNAL(progress(const SyncFileItem &,quint64)), From 5be214d9b99574eb308a6d754d86a3482722581a Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 28 Mar 2017 09:48:45 +0200 Subject: [PATCH 4/5] Make target chunk upload duration an option, not capability --- src/gui/folder.cpp | 40 +++++++++++++++++++++---------- src/gui/folder.h | 2 ++ src/libsync/capabilities.cpp | 9 ------- src/libsync/capabilities.h | 11 --------- src/libsync/configfile.cpp | 13 +++++++--- src/libsync/configfile.h | 1 + src/libsync/discoveryphase.h | 7 ++++++ src/libsync/propagateuploadng.cpp | 2 +- 8 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 4b938157ef0..7c82b6814bd 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -645,7 +645,17 @@ void Folder::startSync(const QStringList &pathList) } setDirtyNetworkLimits(); + setSyncOptions(); + _engine->setIgnoreHiddenFiles(_definition.ignoreHiddenFiles); + + QMetaObject::invokeMethod(_engine.data(), "startSync", Qt::QueuedConnection); + + emit syncStarted(); +} + +void Folder::setSyncOptions() +{ SyncOptions opt; ConfigFile cfgFile; @@ -653,16 +663,22 @@ void Folder::startSync(const QStringList &pathList) opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); - opt._initialChunkSize = qgetenv("OWNCLOUD_CHUNK_SIZE").toUInt(); - if (opt._initialChunkSize == 0) { + QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); + if (!chunkSizeEnv.isEmpty()) { + opt._initialChunkSize = chunkSizeEnv.toUInt(); + } else { opt._initialChunkSize = cfgFile.chunkSize(); } - opt._minChunkSize = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE").toUInt(); - if (opt._minChunkSize == 0) { + QByteArray minChunkSizeEnv = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE"); + if (!minChunkSizeEnv.isEmpty()) { + opt._minChunkSize = minChunkSizeEnv.toUInt(); + } else { opt._minChunkSize = cfgFile.minChunkSize(); } - opt._maxChunkSize = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE").toUInt(); - if (opt._maxChunkSize == 0) { + QByteArray maxChunkSizeEnv = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE"); + if (!maxChunkSizeEnv.isEmpty()) { + opt._maxChunkSize = maxChunkSizeEnv.toUInt(); + } else { opt._maxChunkSize = cfgFile.maxChunkSize(); } @@ -673,14 +689,14 @@ void Folder::startSync(const QStringList &pathList) opt._minChunkSize = qMin(opt._minChunkSize, opt._initialChunkSize); opt._maxChunkSize = qMax(opt._maxChunkSize, opt._initialChunkSize); + QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION"); + if (!targetChunkUploadDurationEnv.isEmpty()) { + opt._targetChunkUploadDuration = targetChunkUploadDurationEnv.toUInt(); + } else { + opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration(); + } _engine->setSyncOptions(opt); - - _engine->setIgnoreHiddenFiles(_definition.ignoreHiddenFiles); - - QMetaObject::invokeMethod(_engine.data(), "startSync", Qt::QueuedConnection); - - emit syncStarted(); } void Folder::setDirtyNetworkLimits() diff --git a/src/gui/folder.h b/src/gui/folder.h index 67026bd52ed..591f2de9e10 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -307,6 +307,8 @@ private slots: void checkLocalPath(); + void setSyncOptions(); + enum LogStatus { LogStatusRemove, LogStatusRename, diff --git a/src/libsync/capabilities.cpp b/src/libsync/capabilities.cpp index 213a36d789f..cee5f1aebe2 100644 --- a/src/libsync/capabilities.cpp +++ b/src/libsync/capabilities.cpp @@ -130,13 +130,4 @@ QList Capabilities::httpErrorCodesThatResetFailingChunkedUploads() const return list; } -quint64 Capabilities::desiredChunkUploadDuration() const -{ - QByteArray value = _capabilities["dav"].toMap()["target_chunk_upload_request_duration_msec"].toByteArray(); - if (!value.isEmpty()) { - return value.toLongLong(); - } - return 0; -} - } diff --git a/src/libsync/capabilities.h b/src/libsync/capabilities.h index 9a3270d013a..e3d8c013c23 100644 --- a/src/libsync/capabilities.h +++ b/src/libsync/capabilities.h @@ -42,17 +42,6 @@ class OWNCLOUDSYNC_EXPORT Capabilities { bool shareResharing() const; bool chunkingNg() const; - /** - * The desired time in ms needed for a single-chunk upload. - * - * The chunk size will be dynamically adjusted to target - * this value. - * - * Capability: dav/target_chunk_upload_request_duration_msec - */ - quint64 desiredChunkUploadDuration() const; - - /// disable parallel upload in chunking bool chunkingParallelUploadDisabled() const; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 11529f2e06a..bad5fd05d0e 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -53,8 +53,9 @@ static const char updateCheckIntervalC[] = "updateCheckInterval"; static const char geometryC[] = "geometry"; static const char timeoutC[] = "timeout"; static const char chunkSizeC[] = "chunkSize"; -static const char minChunkSizeC[] = "minChunkSizeC"; -static const char maxChunkSizeC[] = "maxChunkSizeC"; +static const char minChunkSizeC[] = "minChunkSize"; +static const char maxChunkSizeC[] = "maxChunkSize"; +static const char targetChunkUploadDurationC[] = "targetChunkUploadDuration"; static const char proxyHostC[] = "Proxy/host"; static const char proxyTypeC[] = "Proxy/type"; @@ -135,7 +136,7 @@ quint64 ConfigFile::chunkSize() const quint64 ConfigFile::maxChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(maxChunkSizeC), 50*1000*1000).toLongLong(); // default to 50 MB + return settings.value(QLatin1String(maxChunkSizeC), 100*1000*1000).toLongLong(); // default to 100 MB } quint64 ConfigFile::minChunkSize() const @@ -144,6 +145,12 @@ quint64 ConfigFile::minChunkSize() const return settings.value(QLatin1String(minChunkSizeC), 1000*1000).toLongLong(); // default to 1 MB } +quint64 ConfigFile::targetChunkUploadDuration() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(QLatin1String(targetChunkUploadDurationC), 60*1000).toLongLong(); // default to 1 minute +} + void ConfigFile::setOptionalDesktopNotifications(bool show) { QSettings settings(configFile(), QSettings::IniFormat); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 8191ab13e6e..2f3a6beccc9 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -117,6 +117,7 @@ class OWNCLOUDSYNC_EXPORT ConfigFile quint64 chunkSize() const; quint64 maxChunkSize() const; quint64 minChunkSize() const; + quint64 targetChunkUploadDuration() const; void saveGeometry(QWidget *w); void restoreGeometry(QWidget *w); diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 49c42eb2818..e34e9505e45 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -41,6 +41,7 @@ struct SyncOptions { , _initialChunkSize(10 * 1000 * 1000) // 10 MB , _minChunkSize(1 * 1000 * 1000) // 1 MB , _maxChunkSize(100 * 1000 * 1000) // 100 MB + , _targetChunkUploadDuration(60 * 1000) // 1 minute {} /** Maximum size (in Bytes) a folder can have without asking for confirmation. @@ -63,6 +64,12 @@ struct SyncOptions { /** The maximum chunk size in bytes for chunked uploads */ quint64 _maxChunkSize; + + /** The target duration of chunk uploads for dynamic chunk sizing. + * + * Set to 0 it will disable dynamic chunk sizing. + */ + quint64 _targetChunkUploadDuration; }; diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 733b9eb7d09..afc14ac921f 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -400,7 +400,7 @@ void PropagateUploadFileNG::slotPutFinished() // // Dynamic chunk sizing is enabled if the server configured a // target duration for each chunk upload. - double targetDuration = propagator()->account()->capabilities().desiredChunkUploadDuration(); + double targetDuration = propagator()->syncOptions()._targetChunkUploadDuration; if (targetDuration > 0) { double uploadTime = job->msSinceStart(); From cd5a31766c41c0b2e94ad09bcd40ff8da8c8d9f3 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 28 Mar 2017 09:56:44 +0200 Subject: [PATCH 5/5] Add explanation of averaging in --- src/libsync/propagateuploadng.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index afc14ac921f..300c0eec187 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -404,20 +404,26 @@ void PropagateUploadFileNG::slotPutFinished() if (targetDuration > 0) { double uploadTime = job->msSinceStart(); - auto correctedSize = static_cast( + auto predictedGoodSize = static_cast( _currentChunkSize / uploadTime * targetDuration); - // There can be multiple chunk uploads going on at the same time. - // So don't force the chunk size to the new predicted best size - // and instead move it there gradually. + // 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. + quint64 targetSize = (propagator()->_chunkSize + predictedGoodSize) / 2; + propagator()->_chunkSize = qBound( propagator()->syncOptions()._minChunkSize, - (propagator()->_chunkSize + correctedSize) / 2, + targetSize, propagator()->syncOptions()._maxChunkSize); - qDebug() << "Chunked upload of " << _currentChunkSize << " took " << uploadTime - << " desired is " << targetDuration << ", expected good chunk size is " - << correctedSize << " and nudged next chunk size to " << propagator()->_chunkSize; + qDebug() << "Chunked upload of" << _currentChunkSize << "bytes took" << uploadTime + << "ms, desired is" << targetDuration << "ms, expected good chunk size is" + << predictedGoodSize << "bytes and nudged next chunk size to " + << propagator()->_chunkSize << "bytes"; } bool finished = _sent == _item->_size;