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.

Sign up for GitHub

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

Jump to bottom

[1.8.0] displayed Transfer speed Higher than limit #3095

Closed
cdamken opened this issue Apr 10, 2015 · 40 comments
Closed

[1.8.0] displayed Transfer speed Higher than limit #3095

cdamken opened this issue Apr 10, 2015 · 40 comments
Assignees
@guruz
Labels
blue-ticket Enhancement ReadyToTest QA, please validate the fix/enhancement type:bug
Milestone
2.0 - Multi-account

Comments

@cdamken
Copy link
Contributor

cdamken commented Apr 10, 2015

Expected behavior

Limit upload speed to 50 KB/s
Upload to 50KB/s

Actual behavior

Limit upload speed to 50 KB/s
Upload to 70KB/s
speed

How to reproduce it:

Sync-client 1.8.0
1.- Add a big File to the upload queue (I added a 2 GB file)
2.- Change the transfer sped to 50 KB/s
3.- Pause
4.- Resume
5.- Wait until the speed stabilizes.

Tested with different speeds.
when 5KB/s
upload 8KB/s

@MorrisJobke

00002794
@dragotin dragotin added this to the 1.9 - Multi-account milestone Apr 10, 2015
@dragotin
Copy link
Contributor

@ogoffart mind to check this? Maybe it's just fuzzy calculation in the progress widget.

@ogoffart
Copy link
Contributor

What platform ? (which version of Qt)

CC @guruz

@guruz
Copy link
Contributor

guruz commented Apr 10, 2015

Carlos, have you tried a system tool that displays actual speed? I dont know if windows task manager can do it

@guruz guruz changed the title [1.8.0] Transfer speed is not limited [1.8.0] displayed Transfer speed Higher than limit Apr 10, 2015
@cdamken
Copy link
Contributor Author

cdamken commented Apr 15, 2015

@ogoffart Windows with 1.8.0. How can I find out the Qt version?

@guruz I will test wit another tool, it's hard to see directly in the Windows network notification, but anyway the widget tells another speed.

@ogoffart ogoffart assigned guruz and unassigned ogoffart Apr 15, 2015
@caratsch
Copy link

@guruz the speed looks right on the system (Resource Monitor shows the right speed). The sync client shows now 260 KiB/s, but only 10 KiB/s are allowed and currently transferred.

@cdamken
Copy link
Contributor Author

cdamken commented Apr 15, 2015

@guruz Then the only problem is the speed shown in the widget

@ogoffart
Copy link
Contributor

We probably should remove that speed there, it is totaly not accurate.

@guruz
Copy link
Contributor

guruz commented Apr 15, 2015

...let's make it a rolling average calculation instead of momentary speed display.

@ckamm
Copy link
Contributor

ckamm commented Apr 15, 2015

@guruz I've code for that ready to be merged, pending review. It changes it to an exponential moving average. See #2328. Maybe I'll just merge these to master for testing...

@guruz
Copy link
Contributor

guruz commented Apr 15, 2015

@ckamm Sounds good.. :)

@cdamken
Copy link
Contributor Author

cdamken commented May 20, 2015

Is something fixed in 1.8.1? Should I test it?

@ckamm
Copy link
Contributor

ckamm commented May 20, 2015

@cdamken Not that I'm aware. There are some patches in master (for 2.0).

@guruz
Copy link
Contributor

guruz commented May 20, 2015

@cdamken Can you please try ownCloud-2.0.0.5092-nightly20150520-setup.exe ?

@cdamken
Copy link
Contributor Author

cdamken commented May 26, 2015

@guruz Where can I have it? http://download.owncloud.com/desktop/daily/ here is not available.

@danimo
Copy link
Contributor

danimo commented May 26, 2015

@cdamken build 5110 should show up in that location in a few minutes.

@caratsch
Copy link

@guruz Seems not to be fixed in the build 5110 nightly. At the beginning it starts at about 60 MB/s and goes slowly down until 3.3 KB/s, but it's uploading with 10KB/s the whole time.
capture

@cdamken
Copy link
Contributor Author

cdamken commented Jun 9, 2015

@danimo I just tested the 5110 too, same results like @caratsch

@MTRichards
Copy link

So this will be resolved with the release of 1.8.3?

@dragotin
Copy link
Contributor

Probably not, I am not aware that we pushed something to 1.8 branch. @ckamm anything we can/want to backport?

@ckamm
Copy link
Contributor

ckamm commented Jun 24, 2015

@dragotin I don't think this is something critical or safe enough to backport to 1.8 at this stage.

@danimo danimo removed this from the 2.0.1-next milestone Aug 5, 2015
@cdamken
Copy link
Contributor Author

cdamken commented Aug 5, 2015

@ckamm @danimo IMHO no need to backport, but in 2.0 would nice to have it

@ogoffart
Copy link
Contributor

ogoffart commented Aug 6, 2015

Remember that the thing only show one of the files and there are several items being downloaded in parallel. Usually 3 items.

And 10/3 = 3.3, so it's consistant!

ogoffart added a commit that referenced this issue Aug 6, 2015
@ogoffart
Propagator: do not use parallelism when we have bandwidth limit
b20f29f 
When user wants to limit the bandwidth, he does not care about speed
anymore. And parallelism on slow network might cause problems.

For issue #3382
Will also help for #3095
@ogoffart ogoffart mentioned this issue Aug 6, 2015
Closed
@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

@ogoffart I can't remember something that nobody told me 😄 ... The same thing will happen with the end user that the just see in the list 1 file in the syn-client and the other 2 are hidden.
(It makes a similar confusion with MB and MiB)

How about displaying the 3 files that are being uploaded or downloaded?

guruz added a commit that referenced this issue Aug 6, 2015
@guruz
Progress: Show real speed and all filenames #3403 #3095
1feb749
@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Aug 6, 2015
ogoffart added a commit that referenced this issue Aug 6, 2015
@ogoffart @guruz
Propagator: do not use parallelism when we have bandwidth limit
29d7903 
When user wants to limit the bandwidth, he does not care about speed
anymore. And parallelism on slow network might cause problems.

For issue #3382
Will also help for #3095

(cherry picked from commit b20f29f)
guruz added a commit that referenced this issue Aug 6, 2015
@guruz
Propagator: Re-order statement of previous commit
593aa00 
The user should be allowed to override.
For issue #3382
Will also help for #3095
@guruz
Copy link
Contributor

guruz commented Aug 6, 2015

How about displaying the 3 files that are being uploaded or downloaded?

Will be in next nightly :-) Please test then..

@guruz guruz mentioned this issue Aug 6, 2015
Closed
@guruz
Copy link
Contributor

guruz commented Aug 6, 2015

@cdamken http://dl-oc.owncloud.com/desktop/daily/ownCloud-2.0.0.5361-nightly20150806-setup.exe

@phil-davis
Copy link
Contributor

2.0.0.5361 on Windows - working and showing me the 3 files and a total download speed that is very believable for my link.
After the 3 filenames that are comma-separated there is a dash (or underscore?) followed by the speed (e.g. the 69 kB/s in the screen shot). That "-" has a weird gray-shade vertical bar rendered also.
I wonder where that artifact comes from?
owncloud-syncing-progress-bar

@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

@guruz I need to uninstall the previous version to install the new one?

error

@danimo
Copy link
Contributor

danimo commented Aug 6, 2015

@cdamken That was a one-time glitch. Simply install a new version of the testpilot cloud, and choose not to uninstall the old one after the upgrade.

@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

@danimo Ok

@guruz
Copy link
Contributor

guruz commented Aug 6, 2015

@phil-davis Hmpf, on my OS X those are nice UTF-8 arrows. Why does this not work :-|
https://github.com/owncloud/client/blob/master/src/gui/folderstatusmodel.cpp#L712

Thanks for testing

@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

speed

@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

@guruz I press enter 😄 Is there a 1 before the 3.7kB/s??

Now the 3 files are displayed! 👍

@phil-davis
Copy link
Contributor

Before the "3.7MB/s" it is supposed to be a down-arrow.
I tried in MS Word on this same computer doing Alt-8595 (2193 hex = 8595 decimal) and it displays a nice down-arrow in Calibri font. So Microsoft does at least know how to render that down-arrow in at least 1 font :)

@cdamken
Copy link
Contributor Author

cdamken commented Aug 6, 2015

@phil-davis Yes, that should be an arrow, probably my display does not show it right, it's in a VM

@guruz
Copy link
Contributor

guruz commented Aug 6, 2015

^^ @jturcotte Maybe you know if this is a Qt issue? @danimo claims it is a font issue.

In any case, i'll ifdef something for Windows for now

guruz added a commit that referenced this issue Aug 6, 2015
@guruz
Windows: Don't use UTF-8 arrows #3095
b5edc63
@phil-davis
Copy link
Contributor

This gives an easy way to verify the new "feature" that downloads are serialized when a download bandwidth is specified in the Network settings. I set Download Bandwidth to 20KBytes/s and added a few files directly on the server. The progress display shows them downloading 1 at a time at around 20kB/s. The same works for uploading.
Note that if there is either a download or upload speed limit set, then file transfers are serialized in both directions. e.g. If I set a download bandwidth, and then add a dozen files to the client to upload, then the upload is done 1 file at a time. I think that is probably reasonable.

@guruz
Copy link
Contributor

guruz commented Aug 10, 2015

@cdamken @phil-davis beta1 should not use the UTF-8 arrows on Windows anymore.

@phil-davis
Copy link
Contributor

Confirmed good on 2.0.0beta1 on Windows. The words "upload" and "download" appear in the text rather than the arrows.

@danimo danimo closed this as completed Aug 10, 2015
@cdamken
Copy link
Contributor Author

cdamken commented Aug 10, 2015

👍

@phil-davis
Copy link
Contributor

The "arrows" rendering on Windows has been fixed now by changing it to the words "upload" and "download". 2.0.0beta1 and later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@guruz guruz

Labels
blue-ticket Enhancement ReadyToTest QA, please validate the fix/enhancement type:bug
Projects
None yet
Milestone
2.0 - Multi-account
Development

No branches or pull requests
9 participants
@ckamm @ogoffart @guruz @dragotin @danimo @phil-davis @MTRichards @cdamken @caratsch