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

Desktop: Set keep-alive for WebDAV/Nextcloud sync #4668

Merged
merged 4 commits into from Mar 26, 2021

Conversation

roman-r-m
Copy link
Collaborator

This is a follow up to #4625

I ran a test on Windows 8:

  • Version 1.7.11 - no Keep-Alive
22:56:40: Preparing scheduled sync
...
23:03:08: Operations completed:

~6.5 minutes

  • Dev build with Keep-Alive
21:33:49: Preparing scheduled sync
...
21:37:13: Operations completed

~3 minutes

The sync was pretty fast as the server was on the same network and did not have https. As with Linux, I expect the difference to be much higher with https enabled.

@roman-r-m
Copy link
Collaborator Author

@tessus would you be able to tests this on Mac? I see no reason why it should not work there but can't be 100% sure.

@tessus
Copy link
Collaborator

tessus commented Mar 14, 2021

Yes, I can do that. I'm not sure if I get to it today though.

Did you generate X notes per test run? Was the size of each note the same, or only the sum of these X notes?

@tessus tessus added desktop All desktop platforms sync sync related issue labels Mar 14, 2021
@roman-r-m
Copy link
Collaborator Author

Did you generate X notes per test run? Was the size of each note the same, or only the sum of these X notes?

I just used a dump of my "production" Joplin database. The stats are:

23:03:08: createRemote: 1272
23:03:08: deleteRemote: 6
23:03:08: fetchingTotal: 1273
23:03:08: fetchingProcessed: 1273
23:03:08: Total folders: 27
23:03:08: Total notes: 855
23:03:08: Total resources: 190

Total resources size is ~200 Mb.

@tessus
Copy link
Collaborator

tessus commented Mar 15, 2021

Well, that was a bust. I knew that Joplin's sync performance was bad, but I never thought that it was that bad. Abysmal really.
I can't wait to setup the Joplin server.

My desktop app usually syncs in the background and I sync every 5 minutes, so only a few notes are synced. That's why I've never notived how bad it actually is.

The fact that the desktop clients can't do h2 is a very big issue.

Setup

Nextcloud 19, https

Server: 100 Mbit
Client: 500 Mbit

round-trip min/avg/max/stddev = 118.747/124.031/128.298/3.064 ms

I can download/upload from that server with 10-12 MB. (Although in my apartment I have an upstream cap of 15-20 Mbit.)

Result

Size of notes + resources: 15.2 MB (yes, you've read that correctly)

branch (commit) Start End Duration Seconds
dev (76c143e) 18:15:45 19:41:26 1h 25m 41s 5141
webdav-keepalive (dabfd06) 19:48:35 21:37:28 1h 48m 53s 6533

dev (76c143e)

2021-03-14 19:41:26: "Sync: finished: Synchronisation finished [1615760145856]"
2021-03-14 19:41:26: "Operations completed: "
2021-03-14 19:41:26: "createRemote: 910"
2021-03-14 19:41:26: "fetchingTotal: 910"
2021-03-14 19:41:26: "fetchingProcessed: 910"
2021-03-14 19:41:26: "Total folders: 69"
2021-03-14 19:41:26: "Total notes: 674"
2021-03-14 19:41:26: "Total resources: 167"

webdav-keepalive (dabfd06)

2021-03-14 21:37:28: "Sync: finished: Synchronisation finished [1615765715437]"
2021-03-14 21:37:28: "Operations completed: "
2021-03-14 21:37:28: "createRemote: 910"
2021-03-14 21:37:28: "fetchingTotal: 910"
2021-03-14 21:37:28: "fetchingProcessed: 910"
2021-03-14 21:37:28: "Total folders: 69"
2021-03-14 21:37:28: "Total notes: 674"
2021-03-14 21:37:28: "Total resources: 167"

@tessus
Copy link
Collaborator

tessus commented Mar 15, 2021

@roman-r-m
Copy link
Collaborator Author

Thanks for testing!

Well, that was a bust. I knew that Joplin's sync performance was bad, but I never thought that it was that bad. Abysmal really.
I can't wait to setup the Joplin server.

I think there's some room for improvement for WebDAV and others, but that's another conversation.

It's quite surprising that this change made things worse on MacOS.
I guess I'll just make this change only for Linux and Windows.

@roman-r-m
Copy link
Collaborator Author

Also just noticed that there's also a method to check for FreeBSD! Wonder if anyone is using Joplin there.

@roman-r-m roman-r-m changed the title Desktop: Set keep-alive for all desktop systems, not just for Linux Desktop: Set keep-alive on Windows Mar 15, 2021
@tessus
Copy link
Collaborator

tessus commented Mar 15, 2021

I think there's some room for improvement for WebDAV and others, but that's another conversation.

That's an understatement.

Syncing 15.2 MB in 1.45 - 1.8 hours is not usable. I'm shocked. Really. I am actually speechless.
I guess I'm gonna need a few days to process this.

@roman-r-m
Copy link
Collaborator Author

Syncing 15.2 MB in 1.45 - 1.8 hours is not usable. I'm shocked. Really. I am actually speechless.

Not sure why but this seems much slower than my test on Linux. Took me 1hr to sync 200Mb to a 3rd party (i.e. not on a local network) WebDAV instance with https enabled.
Still slow but an order of magnitude faster.

@tessus
Copy link
Collaborator

tessus commented Mar 15, 2021

I have no idea. I can create a test account on my Nextcloud, if necessary or wanted.

@tessus
Copy link
Collaborator

tessus commented Mar 15, 2021

I doubt it's my server though. In the Nextcloud web UI I can upload a 16 MB file in 25s (my upstream bandwidth sucks), but the download is under 2s.

@roman-r-m
Copy link
Collaborator Author

I have no idea. I can create a test account on my Nextcloud, if necessary or wanted.

Definitely not necessary. I'm curious but probably won't have time to check within the next few weeks.

@laurent22
Copy link
Owner

laurent22 commented Mar 16, 2021

There's something wrong with either your test, laptop or server @tessus because that's just ridiculous. Of course it doesn't take 90 minutes to sync 15 MB. I have 700MB and it doesn't take 3 days to sync it with Nextcloud, more like 1h.

Anyway, I'd be more interested in knowing if it works with mobile. Did you have the opportunity to check @roman-r-m?

@roman-r-m
Copy link
Collaborator Author

This change is only for desktop, I will take a look at mobile (Android only really)

@tessus
Copy link
Collaborator

tessus commented Mar 16, 2021

There's something wrong with either your test, laptop or server @tessus

I don't see how, @laurent22. I ruled everything out, as you can see from my comments. If I can download a 16MB file via Nextcloud in less than 2s and upload the same file in 25s to Nextcloud, I ruled out network issues for the client and the server. The upload speed is that "bad" because of the the 20Mbit upstream limit from my Internet provider. It does not explain the horrible sync time though. I ran tests from a VM in Azure and AWS and the download and upload maxes out at 12 MB/s for a file transfer. (No server issue.)

I also added the ping latency and a speedtest. Btw, I can run the speedtest and a file upload/download while the sync is in progress. No difference.

I also offered a test user that you can run these tests yourself.

The test looked like this.

  • Start a Joplin dev instance with a new profile
  • configure sync target Nextcloud
  • sync
  • sync
  • import jex files with 910 items (15.2 MB)
  • record start time
  • sync
  • record end time

Of course I used different paths for the sync target for each test.

I've been in DB2 performance for many years, so you can be assured that I do know how to run performance tests.

log_sync1.txt.gz
log_sync2.txt.gz

@roman-r-m
Copy link
Collaborator Author

I also offered a test user that you can run these tests yourself.

Can I get one? Sounds like an interesting challenge to see what's going on there.

@laurent22
Copy link
Owner

Ok not sure what's going on then. 910 items would not be that fast to upload anyway because, unlike download it's not parallelised, but for example in my recent test it took 24 min for 744 items with Nextcloud - https://discourse.joplinapp.org/t/joplin-server-pre-release-is-now-available/13605

@tessus
Copy link
Collaborator

tessus commented Mar 17, 2021

Laurent, the mystery is solved.

Nextcloud was and is the problem. The WebDAV access and what they do in the background when you access a file via webdav is extremely slow.

I just synced the same data as yesterday in 17m 1s (1021 s) via normal WebDAV.

This is just mind-boggling. I'm using Apache with event MPM, php-fpm (7.4) + OpCache, MySQL8 (via socket), Redis (via socket) (memcache.locking), APCu (memcache.local). The entire MySQL database fits into memory. All Nextcloud php files are compiled and cached in the OpCache.
There's nothing I could optimize to make the Nextcloud/WebDAV any faster. I have no idea what the heck these Nextcloud devs are doing.

@laurent22
Copy link
Owner

Indeed that's strange. Perhaps the PHP opcache is somehow not working or not properly enabled in the ini file? I've always assumed that the big reason Nextcloud is slow was because WebDAV in general is slow, but it looks like Nextcloud adds some overhead too.

@roman-r-m
Copy link
Collaborator Author

There's nothing I could optimize to make the Nextcloud/WebDAV any faster. I have no idea what the heck these Nextcloud devs are doing.

I think it may be a problem with your Nextcloud or server config. Here #4625 (comment) I synced 200Mb in 30 minutes (with keep-alive) to https://cloud.woelkli.com (so not on my local network).

Still slow but not as slow as with your server.

@tessus
Copy link
Collaborator

tessus commented Mar 17, 2021

Perhaps the PHP opcache is somehow not working or not properly enabled in the ini file?

Nope, it is working and the Nextcloud PHP files are in the cache.

I think it may be a problem with your Nextcloud or server config.

It's possible, but I wouldn't know where. I use all caches available (Redis, APCu, OpCache), everything fits into memory (even the MySQL database), I use sockets to not even have a TCP overhead for local connections, I use self-compiled Apache and php binaries (including their dependencies). I really don't see where I could have screwed up.

No matter, I'm moving the sync target to native WebDAV and be done with it.

@roman-r-m
Copy link
Collaborator Author

Laurent, I can confirm that on Android we set Keep-Alive.
Can it be merged at least for Windows then?

@tessus
Copy link
Collaborator

tessus commented Mar 17, 2021

IMO Keep-Alive should be set on all platforms by default. There's no logical reason to limit this to certain platforms. IP stacks should follow a standard. There's nothing bad in setting Keep-Alive header.

@roman-r-m
Copy link
Collaborator Author

I agree. Just being overly cautious here. I'd much rather have it enabled for all platforms.

@roman-r-m roman-r-m changed the title Desktop: Set keep-alive on Windows Windows: Set keep-alive for WebDAV/Nextcloud sync Mar 19, 2021
@tessus
Copy link
Collaborator

tessus commented Mar 20, 2021

Laurent, was there a reason why Keep-Alive was only set for a specific OS? I don't see a downside adding this for all systems.

@laurent22
Copy link
Owner

I guess we could enable keep-alive for all platforms but keeping in mind it would be a bit of a gamble. WebDAV implementations, network stacks and libraries have glitches and any change like this one might trigger them. As an example I've spent several hours on this React Native bug because suddenly their fetch library was requiring the Content-Type header to be set and the only error message was "Network request failed". See also the "JoplinIgnore-" header hack to go around a bug in the iOS network stack.

There's no logical reason to limit this to certain platforms. IP stacks should follow a standard.

They should but they don't and that's what we need to work with.

So let's try to enable it on all platforms and hopefully if there's a regression in some contexts we'll be able to know it's due to this parameter.

@laurent22
Copy link
Owner

@roman-r-m, so could you enable it on all platform then please? And you say it was already enabled on Android/iOS? Or is it something that needs to be done?

@roman-r-m
Copy link
Collaborator Author

so could you enable it on all platform then please?

This is done.

And you say it was already enabled on Android/iOS? Or is it something that needs to be done?

Enabled on Android - I have verified with tcpdump. However, I do not know enough of iOS to confirm.

@roman-r-m roman-r-m changed the title Windows: Set keep-alive for WebDAV/Nextcloud sync Desktop: Set keep-alive for WebDAV/Nextcloud sync Mar 23, 2021
@laurent22
Copy link
Owner

Enabled on Android - I have verified with tcpdump. However, I do not know enough of iOS to confirm.

Oh I see, you mean it's already built-in? Not something we need to manually enable?

@roman-r-m
Copy link
Collaborator Author

Oh I see, you mean it's already built-in? Not something we need to manually enable?

Exactly. It's part of OkHttp library

@laurent22
Copy link
Owner

Ok let's give it a try then, thanks for looking into it Roman.

@laurent22 laurent22 merged commit 49e6b5c into laurent22:dev Mar 26, 2021
@roman-r-m roman-r-m deleted the webdav-keepalive branch March 26, 2021 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms sync sync related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants