Prototype: dynamic chunkingNG #5368

mrow4a · 2016-12-08T00:25:47Z

In current implementation, big files e.g. 100MB are being chunked into smaller pieces called 10MB chunks. The problem here is that this is fixed value, and it is not appropriate on all types of the networks (WiFi versus FiberToTheHome versus ETHERNET LAN). For WiFi it makes sense for small chunks, while for fast networks, it makes sense to have very big chunks - optimal maximum of 50MB.

Server capability

This PR proposes implementation, which in first phase downloads from the server the following capabilities:

	/*
	 * This function will return:
	 *
	 * - <chunking>: version number of chunking on the client
	 *
	 * - <max_single_upload_request_duration_msec>: 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 on the current client version to sync 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 <max_single_upload_request_duration_msec>
	 *
	 * 		Dynamic chunking client algorithm is specified in the ownCloud documentation and uses <max_single_upload_request_duration_msec> to estimate if given
	 * 		bandwidth allows higher chunk sizes (because of high goodput)
	 */
	public function getCapabilities() {
		return [
			'dav' => [
				'chunking' => '1.0',
				'max_single_upload_request_duration_msec' => '10000',
			]
		];
	}

The above parameter will be used as a reference in client learning of the available host/server bandwidth and capabilities of the server.

How dynamic chunking works

Lets assume, that the client wants to synchronise 100MB file. Lets assume, that request RTT is 100ms. This means, that it takes around 100ms to send 1st chunk with 1kB of the data. Recalling, the 1st to nth chunked PUTs will not cause any delay in database (as single file PUTs are doing). Final MOVE will cause file to be added to the ownCloud cache.

In the current implementation, the following will happen, taking around 3,6 second:

File will be equally divided into 10 chunks of 10MB, send to assemply stream and MOVEd (with bookkeeping operation) to ownCloud.

In the PRed implementation, the following will happen, taking around 2,5second (30% faster):

What happend, is that 1 PUT carries 10MB, 2nd PUT carries 20MB, 3rd PUT carries 30MB, and 4th PUT carries 40MB.

Algorithm description

Algorithm is based on changes in request time to the reference value, and being corrected using ln(change)-1.
This results in the following - please mind that this does not includes TCP Congestion Control and in reality growth will be lower:

Three colors are showing the 4 consequitive PUTs for static bandwidth of 10MB/s, chunk sizes 10MB->20MB->25MB->30MB, with slowing growth or reset to 10MB again.

The above graph presents what will be the next values of chunking sizes for the specific static congestion window and specific values of previous values of chunking sizes.

Higher the bandwidth, faster will approach maximum value of 50MB. For lower bandwidths it will not rise at all or rise very slowly.

Higher the chunking size, higher bandwidth needed to cause growth.

You also probably ask yourselves, ok, but this will not work for higher latencies (RTTs).

Yes it works, but until the specific limit of 1s latency. With increasing latency above 1s it will decrease exponentialy. However, this is never the case, since such latencies are rare and are signaling an error. This kind or RTTs are also possible for database bookkeeping operation, however, this is not a case here, since MOVE is a separate operator here, and this one depends on the server.

Why Natural Logarithm?

Natural logarithm has a property that for given given x, when x<1, logarithm is negative, x=1, logarithm is 0, and for x>1, logarithm is possitive.

If your x is ratio of CurrentRequestDuration / ReferenceRequestDuration, when CurrentRequestDuration is equal to ReferenceRequestDuration, the next chun size value ''_lastChunkSize + log(CurrentRequestDuration / ReferenceRequestDuration)*chunkSize()" is equal to "_lastChunkSize".

As an example, given network 1MBs, thus 10MB default chunk will take 10s, and having reference value equal to 10s, our change will be exactly 0.

Next property of logarithm is that, if you the result of ''log()'' you substract 1, you shift the value when logarithm becomes possitive e.g. given network 10MBs, thus 10MB default chunk will take 1s, and having reference value equal to 10s, our correction parameter ''log()-1'' change will be equal to 1, so your next chunk size will be 20MBs. The zero border will be shifted to 3MBs in order to start increasing the chunk size. This can be shifted by substracting log value more.

RFC - Request for Comments
@DeepDiver1975 @ogoffart @guruz @jturcotte @hodyroff @felixboehm @butonic

Excel with calculations:
dynamic-chunking.ods.zip

mention-bot · 2016-12-08T00:25:48Z

@mrow4a, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ogoffart, @guruz and @ckamm to be potential reviewers.

mrow4a · 2016-12-08T00:38:58Z

src/libsync/propagateuploadng.cpp

+            // TODO: give link to documentation
+            if (log>0){
+                currentChunkSize = qMin(_lastChunkSize + (qint64) log*chunkSize(), maxChunkSize());
+            }


The above lines are core algorithm implementation (yes, it is simple). Core thing is correction parameter NaturalLogarithm(change)-1

Possitive correction parameter will cause chunk size growth, while negative parameter (signal of limit) will cause drop to defautt chunk size of 10MB. And process starts again.

mrow4a · 2016-12-08T01:39:39Z

I would love to use the same mechnism for Bundling #5319

mrow4a · 2016-12-08T09:07:50Z

src/libsync/propagateuploadng.cpp

+    quint64 currentChunkSize = chunkSize();
+
+    // this will check if getRequestMaxDurationDC is set to 0 or not
+    double requestMaxDurationDC = (double) getRequestMaxDurationDC();


This actually checks if capability is turned on or not

mrow4a · 2016-12-08T09:08:45Z

src/libsync/propagateuploadng.cpp

+    }
+
+    // prevent situation that chunk size is bigger then required one to send
+    currentChunkSize = qMin(currentChunkSize, fileSize - _sent);


this is main algorithm

ogoffart · 2016-12-08T11:03:18Z

So this needs a change in the server?

mrow4a · 2016-12-08T11:06:11Z

No, it not even needs the capability from the server, it is enought that you set this 10s - so time to upload 10MB on 1MB/s link on the client. But better if this will be in control of sys admin..

On the server, I just addded capability as in the beggining of the post

guruz · 2016-12-08T12:48:05Z

src/libsync/propagateupload.h

+    * 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:


MORD means MURDER in German :)

guruz · 2016-12-08T12:49:10Z

src/libsync/propagateuploadng.cpp

+        // 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


Please leave it "by step". The compiler will anyway optimize. It is very very very important that we also understand this code in some months.

guruz · 2016-12-08T12:51:01Z

Please mention #4875 in the commit message.

taking around 2,5second (30% faster):

Cool! :)

I would love to use the same mechnism for Bundling #5319

Do you mean for the amount of files in a bundle? Let's not complicate the code too much for now please.. the goal is to get something in a solid state.

mrow4a · 2016-12-08T13:16:25Z

@guruz I think we might need to do that, so the bundle will adjust to the server. Otherwise you will give the same amount of files in bundle both for raspery PI and high performance server and high scale server with high write time.

Bundle needs to know how much it can fit for specific server and bandwidth, otherwise request will take ages like with big files on slow network.

ckamm

This looks good! I have left a bunch of detail comments.

ckamm · 2016-12-13T09:43:37Z

src/libsync/propagateuploadng.cpp

+            // 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 (log>0){


Where's log assigned? This looks like a bug.

Sorry, I commit --amend the PR, to make the variables more descriptive and did not notice that, will change it now, this should be correctionParameter

ckamm · 2016-12-13T09:44:18Z

src/libsync/propagateuploadng.cpp

+            // motivation for logarithm is specified in the dynamic chunking documentation
+            // TODO: give link to documentation
+            if (log>0){
+                currentChunkSize = qMin(_lastChunkSize + (qint64) correctionParameter*chunkSize(), maxChunkSize());


qBound between minChunkSize, newChunkSize and maxChunkSize, otherwise this could be 0 or negative.

Is this state possible if we never get lower? We only increase, starting from default chunk size. If measurement negative, rollback to default chunk size.

ckamm · 2016-12-13T09:54:23Z

src/libsync/propagateuploadng.cpp

+            double requestDuration = (double) _stopWatch.addLapTime(QLatin1String("ChunkDuration")) - lastChunkLap;
+
+            // calculate natural logarithm
+            double correctionParameter = log(requestMaxDurationDC / requestDuration) - 1;


I don't understand the -1. Can you explain again?

To me, this looks odd. log(a/b) -1 is the same as log(a/(b*e)) so it looks like you're dividing requestMaxDurationDC by e. That means that if requestMaxDuration is 10s and the requestDuration is 5s, the correctionParameter would be negative and you'd be reducing the chunk size.

You never reduce the chunking size. You either increase or roll-back to default size.

I see! With the if (correctionParameter > 0) check this means that you only adjust the chunk size upwards if actual_time < target_time / e. I guess that's okay.

The current code never adjusts the chunk size downwards, correct?

(Please also add your replies that @ckamm had asked as code comment)

ckamm · 2016-12-13T09:58:29Z

src/libsync/propagateupload.h

+    * Dynamic chunking client algorithm is specified in the ownCloud documentation and uses <max_single_upload_request_duration_msec> to estimate if given
+    * bandwidth allows higher chunk sizes (because of high goodput)
+    */
+    quint64 _requestMaxDuration;


I think "requestMaxDuration" is a bit misleading, it could be interpreted as requests being aborted it they take too long.

What about "targetRequestDuration" to indicate that this is the duration that the chunk size is calibrated towards?

ckamm · 2016-12-13T10:00:40Z

src/libsync/propagateupload.h

+    * > 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 <max_single_upload_request_duration_msec>


I don't understand how this paragraph relates to the previous one. (or what it is an example of)

ckamm · 2016-12-13T10:01:03Z

src/libsync/propagateupload.h

+    * Exemplary, syncing 100MB files, with chunking size 10MB, will cause sync of 10 PUT requests which max evaluation was set to <max_single_upload_request_duration_msec>
+    *
+    * Dynamic chunking client algorithm is specified in the ownCloud documentation and uses <max_single_upload_request_duration_msec> to estimate if given
+    * bandwidth allows higher chunk sizes (because of high goodput)


minor: throughput

Yes,http://ethancbanks.com/2015/03/06/what-is-the-difference-between-throughput-goodput/. It is the meaning how you understand that. Will change to throughtput to cause less confusion

Oh! I hadn't heard of that term, thanks for the link!

ckamm · 2016-12-13T10:06:15Z

src/libsync/propagateupload.h

@@ -290,23 +290,48 @@ 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 a good start, but it would be nice if this was shared between PropagateUploads, so they don't have to re-learn for each upload.

Maybe BandwidthManager could learn the information.
Although the BandwidthManager is more for limiting currently

I am not fully sure how Bandwidth Manager works, I did not touch it. Of course, we can play with it and test what gives the enhancement easier.

Don't worry about it then. I just meant as a place where you could store a cross-file estimated value fro chunk size

mrow4a · 2016-12-13T10:27:18Z

@ckamm Please check now. I commit --ammend the PR again, now should be ok. Sorry I had changed variables to be more descriptive and did not notice that it broke the condition check.

You can verify how it works just adding the one line to capabilities on the server side.

mrow4a · 2016-12-13T10:37:40Z

@ckamm

qBound between minChunkSize, newChunkSize and maxChunkSize, otherwise this could be 0 or negative.

Is this state possible if we never get lower? We only increase, starting from default chunk size. If measurement negative, rollback to default chunk size.

ckamm · 2016-12-13T12:54:44Z

@mrow4a Yep, the qBound is unnecessary if you only ever adjust the chunk size upwards.

ogoffart · 2016-12-14T12:38:05Z

I suggest this simple algorithm:

  chunkSize = qBound(minValue,  previousChunkSize * targetTime / previousChunkTime  , maxValue)

Note that, (esp. for fast network) the time is bounded by the PHP, and increasing the chunk size will not make the request slower. Meaning we will only converge assymtotically towards the target

For this reason, it is important that the target time is much bigger than the time needed for the PHP to proccess. Otherwise we would always converge quite fast to the minimum value, which would be the worst outcome.

mrow4a · 2017-01-02T14:10:42Z

Should we allow this to work with both ChunkingNG and ChunkingV1?

guruz · 2017-01-03T12:13:26Z

Should we allow this to work with both ChunkingNG and ChunkingV1?

No, only new chunking. The old chunking cannot easily support dynamic chunk sizes, don't change what works :)

ckamm · 2017-01-10T12:40:10Z

@mrow4a Is there something specific you need to make progress on this?

mrow4a · 2017-01-10T15:01:49Z

I need time, https://cs3.surfsara.nl/ (I need to do tests and measurements) and exam session is coming :> I think I also did my work hours for this month.

ckamm · 2017-01-11T09:31:20Z

@mrow4a Understood! All the best with exams!

ckamm · 2017-03-24T14:09:43Z

@mrow4a @guruz @ogoffart I've taken the liberty of taking most feedback and cleaning up this commit. It's pushed as a follow-up fixup commit. The whole thing is not yet rebased onto master (that's why CI can't build it).

Please re-review. It looks good from my point of view now!

ogoffart · 2017-03-27T11:00:34Z

src/libsync/propagateuploadng.cpp

+        // and instead move it there gradually.
+        _propagator->_chunkSize = qBound(
+                _propagator->minChunkSize(),
+                (_propagator->_chunkSize + correctedSize) / 2,


Why take the average and not just set correctedSize ?

The comment speaks about multiple chunk upload going on, but i don't see how this is related.

Both work. My reasoning was this:

The whole thing is heuristic. It's possibly that the code is executed after a single chunk finishes uploading when nothing else was going on, but it could also be that several chunks were competing for bandwidth when we get here. If we wanted to target a specific duration per chunk upload this would not give us the right result even if network throughput was completely constant.

Thus I expect correctedSize to over and undershoot a good size all the time. Using an exponential moving average like this is a cheap way of smoothing chunk sizes a bit. We expect them to fluctuate around "the best" value.

I could put this reasoning into the code or just remove the averaging, both are fine with me.

ogoffart · 2017-03-27T11:06:34Z

(I don't like the fact that there is a dependency between the config file and the sync engine. But since it is already the case, i can't really stop this commit for it. Anyway, it would be nice if it was part of the "SyncOptions" struct instead

ckamm · 2017-03-27T12:15:12Z

@ogoffart I had to rebase on top of master to make SyncOptions available.

ogoffart

Looks good

ckamm · 2017-03-28T09:35:51Z

Merged manually as 53c5f03

mrow4a commented Dec 8, 2016

View reviewed changes

mrow4a force-pushed the dynamic_chunking branch 2 times, most recently from 769cd9e to 0f74164 Compare December 8, 2016 01:33

mrow4a commented Dec 8, 2016

View reviewed changes

mrow4a mentioned this pull request Dec 8, 2016

Prototype: Requests bundling feature implementation #5319

Closed

10 tasks

guruz reviewed Dec 8, 2016

View reviewed changes

guruz added the Performance label Dec 8, 2016

guruz added this to the 2.3.0 milestone Dec 8, 2016

guruz requested a review from ogoffart December 8, 2016 12:51

ckamm suggested changes Dec 13, 2016

View reviewed changes

mrow4a force-pushed the dynamic_chunking branch from 0f74164 to 54b94ff Compare December 13, 2016 10:26

mrow4a mentioned this pull request Dec 18, 2016

In the mixed file size scenario bandwidth is very underutilized - enterprise setup #5391

Open

mrow4a changed the title ~~dynamic chunkingNG prototype~~ Prototype: dynamic chunkingNG Dec 20, 2016

mrow4a mentioned this pull request Jan 8, 2017

Group requests to ensure balance between bandwidth utilization and bookkeeping #5391 #5390 #4498 #1633 #4454 #5440

Closed

4 tasks

guruz modified the milestones: 2.4.0, 2.3.0 Jan 19, 2017

ogoffart reviewed Mar 27, 2017

View reviewed changes

mrow4a and others added 3 commits March 27, 2017 13:43

dynamic chunking prototype

88e0f97

Various fixes and improvements

0468f44

Put min/max chunk sizes into SyncOptions

6aad228

ckamm force-pushed the dynamic_chunking branch from 1e9c5b1 to 6aad228 Compare March 27, 2017 12:14

ckamm added 2 commits March 28, 2017 09:48

Make target chunk upload duration an option, not capability

5be214d

Add explanation of averaging in

cd5a317

ogoffart approved these changes Mar 28, 2017

View reviewed changes

ckamm approved these changes Mar 28, 2017

View reviewed changes

ckamm closed this Mar 28, 2017

ogoffart deleted the dynamic_chunking branch January 25, 2018 12:58

Prototype: dynamic chunkingNG #5368

Prototype: dynamic chunkingNG #5368

Conversation

mrow4a commented Dec 8, 2016 • edited Loading

mention-bot commented Dec 8, 2016

mrow4a Dec 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrow4a commented Dec 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogoffart commented Dec 8, 2016

mrow4a commented Dec 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guruz commented Dec 8, 2016

mrow4a commented Dec 8, 2016 • edited Loading

ckamm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrow4a Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckamm Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrow4a Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrow4a commented Dec 13, 2016 • edited Loading

mrow4a commented Dec 13, 2016

ckamm commented Dec 13, 2016

ogoffart commented Dec 14, 2016

mrow4a commented Jan 2, 2017

guruz commented Jan 3, 2017

ckamm commented Jan 10, 2017

mrow4a commented Jan 10, 2017 • edited Loading

ckamm commented Jan 11, 2017

ckamm commented Mar 24, 2017 • edited Loading

Choose a reason for hiding this comment

ckamm Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

ogoffart commented Mar 27, 2017

ckamm commented Mar 27, 2017

ogoffart left a comment

Choose a reason for hiding this comment

ckamm commented Mar 28, 2017

mrow4a commented Dec 8, 2016 •

edited

Loading

mrow4a Dec 8, 2016 •

edited

Loading

mrow4a commented Dec 8, 2016 •

edited

Loading

mrow4a commented Dec 8, 2016 •

edited

Loading

mrow4a Dec 13, 2016 •

edited

Loading

ckamm Dec 13, 2016 •

edited

Loading

mrow4a Dec 13, 2016 •

edited

Loading

mrow4a commented Dec 13, 2016 •

edited

Loading

mrow4a commented Jan 10, 2017 •

edited

Loading

ckamm commented Mar 24, 2017 •

edited

Loading

ckamm Mar 27, 2017 •

edited

Loading