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

Concurrent PROPFINDs result in error 500 #13705

Closed
davivel opened this issue Jan 27, 2015 · 22 comments · Fixed by #13772
Closed

Concurrent PROPFINDs result in error 500 #13705

davivel opened this issue Jan 27, 2015 · 22 comments · Fixed by #13772

Comments

@davivel
Copy link

davivel commented Jan 27, 2015

Steps to reproduce

  1. Create a folder /c/a/ in your account (not sure if meaningful).
  2. Run
for (( c=1; c<=5; c++ )) 
do 
curl --user <user>:<pass> -X PROPFIND -o out${c}.txt -H "Content-Type: text/xml" -d "<?xml version='1.0' encoding='utf-8' ?><D:propfind xmlns:D='DAV:'><D:allprop/></D:propfind>" http://<domain/path>remote.php/webdav/c/a/ &
done

Expected behaviour

All requests should finish with HTTP code 207; files out1.txt to out5.txt should contain the properties of the folder /c/a/, something like:

<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>[edited]/remote.php/webdav/c/a/</d:href>
      <d:propstat>
        <d:prop>
          <oc:id>00000044ocee3337d5cf</oc:id>
          <oc:permissions>RDNVCK</oc:permissions>
          <d:getetag>"54c75bef2d6bc"</d:getetag>
          <d:getlastmodified>Tue, 27 Jan 2015 09:35:43 GMT</d:getlastmodified>
          <d:resourcetype><d:collection/></d:resourcetype>
          <d:quota-used-bytes>0</d:quota-used-bytes>
          <d:quota-available-bytes>6199443456</d:quota-available-bytes>
        </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

Actual behaviour

4 of 5 methods finish in 500 Internal Error.

Server configuration

ownCloud version: - 8.0.0.4

daily/master

Logs

ownCloud log (data/owncloud.log)

One message

An exception occurred while executing 'UPDATE "oc_preferences" SET "configvalue" = ? WHERE "userid" = ? AND "appid" = ? AND "configkey" = ? ' with params [1422353148, "<user>", "login", "lastLogin"]: SQLSTATE[HY000]: General error: 5 database is locked

is shown per failed request.

Extra

Not only affecting PROPFIND requests, Android client is receiving 500 Internal error when downloads (GET) and refresh (PROPFIND) are run in parallel .

@davivel
Copy link
Author

davivel commented Jan 27, 2015

Extra 2: this is no problem for stable7.

@LukasReschke
Copy link
Member

Solution: Don't use SQLite.

@DeepDiver1975 Related to our recent discussions with @ogoffart and Co.

@davivel
Copy link
Author

davivel commented Jan 27, 2015

Solution: Don't use SQLite.

Sounds fast and clean :)

@karlitschek
Copy link
Contributor

Guys. This is a bug. There is absolutely no technical reason why ownCloud should return a 500 here. The database should be properly locked and the second request should be answered as soon as the first one is done. This is how ownCloud worked for years.
@LukasReschke @DeepDiver1975
Thanks you

@DeepDiver1975
Copy link
Member

Guys. This is a bug. There is absolutely no technical reason why ownCloud should return a 500 here.

indeed - the question is how to solve this and at the end what will be the user experience.

The by-design--behavior of sqlite being not concurrent accessible will result in all operations of all users and all clients to be executed sequentially which will be observed as "ownCloud is damn slow"

Let me see if there is a solution to the database locked issue (FYI: drupal is struggling with this issue for 4 years now with no solution at hand - need to take a closer look - https://www.drupal.org/node/1120020).

My suggestion regarding the user experience would be to disable the webdav interface and/or sync in case sqlite is used. To protect us and the user base.

@guruz
Copy link
Contributor

guruz commented Jan 28, 2015

The by-design--behavior of sqlite

...if the busy-wait comes back in as per #13615 (that used to be in oc5 but then got kicked out)

More context for @karlitschek in owncloud/client#2743

Additionally toning down parallelism also helps with timeouts where e.g. a long running move takes several minutes.

@MTRichards
Copy link
Contributor

@carlaschroder Add a release note please about downloading folders in SQLite with Android as a known issue.

And discouraging the use of SQLite in production.

@karlitschek
Copy link
Contributor

We should fix the issue before we discourage the usage of a database.
There is a potential fix here: #13752
Please test

@PVince81
Copy link
Contributor

This issue is still happening. The mentionned fix is only for files.

The problem above is when updating the config concurrently.
It seems that all 5 requests are authenticating and updating the oc_preferences table at the same time to update the "lastlogin" value and locking each other out.
It seems that the first request goes through but the others fail directly.

I suspect there might not be any lock timeout in place which might explain why it directly fails.
I'll try with #13615

@PVince81
Copy link
Contributor

(the config option for the timeout)

@PVince81
Copy link
Contributor

Another possible fix is to fail gracefully (ignore the error) if the "lastlogin" value cannot be updated due to locking.

@PVince81
Copy link
Contributor

Okay, I tried ignoring the "lastlogin" exception and the propfinds all go through. So at least we have a quickfix for this specific situation.

Next step: try the timeout option.

@PVince81
Copy link
Contributor

No luck with #13615, needs further research

@davivel
Copy link
Author

davivel commented Jan 29, 2015

So, if all the requests were reusing the same session cookie, all the the PROPFINDs would work?

@rperezb , seems a reason to finally enable the cookie support.

@PVince81
Copy link
Contributor

Yes, that's another "workaround".

@rperezb
Copy link

rperezb commented Jan 29, 2015

@davivel great for next version expected this sprint

@PVince81
Copy link
Contributor

Here it is: https://github.com/owncloud/core/blob/master/lib/private/allconfig.php#L180

Unclosed cursor, PR is on the way

@PVince81
Copy link
Contributor

Fix is here: #13772

@MorrisJobke
Copy link
Contributor

Thanks @davivel for this nice to test script 👍

@MorrisJobke MorrisJobke added this to the 8.0-current milestone Jan 29, 2015
@davivel
Copy link
Author

davivel commented Jan 30, 2015

Thanks, guys. Android app working again as expected.

@guruz
Copy link
Contributor

guruz commented Jan 30, 2015

@davivel Wondering..does the Android client do multiple PROPFINDs (or other requestz) that all re-authenticate? It does not use cookies to keep the auth?

@davivel
Copy link
Author

davivel commented Jan 30, 2015

At this moment, it does.

But right now we only need to change a flag to fix that. Next version (coming soon) should use cookies.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants