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

Distinguish 503 Storage not available from 503 Service unavailable #1923

Closed
PVince81 opened this issue Jul 2, 2014 · 16 comments
Closed

Distinguish 503 Storage not available from 503 Service unavailable #1923

PVince81 opened this issue Jul 2, 2014 · 16 comments
Assignees
Labels

Comments

@PVince81
Copy link
Contributor

PVince81 commented Jul 2, 2014

See owncloud/core#9376

From the introduction of server to server sharing, it can happen that a specific mount point is not available so any operation on that folder would return "503 Storage not available".

Currently the sync client shuts down itself (gray) and stops syncing, believing that the whole server is down.

@ogoffart suggested this needs to be fixed in a later version of the sync client.

@guruz
Copy link
Contributor

guruz commented Aug 29, 2014

@ogoffart As discussed also make sure that 503 is never blacklisted as it is supposed to be the temporary answer from the server.

@dragotin
Copy link
Contributor

We shouldn't probably blacklist 5xx bugs at all. Currently we do not very much verify the http error code, see src/libsync/owncloudpropagator.cpp, line 94, it blacklists everything.

@guruz
Copy link
Contributor

guruz commented Aug 31, 2014

@dragotin I remember some months ago we had the same discussion and @ogoffart and you said that 500 should not be blacklisted because the server might go bonkers on a file and then it would be re-uploaded all the time.

@DeepDiver1975 @PVince81 Do you have an opinion on this?
I also favor not blacklisting 5xx at all.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 1, 2014

Can you remind us what the purpose of the blacklist is ?
There's another ticket where people asked for removing it completely.

From what I understand it's mostly to avoid retrying the same broken file over and over again and annoy the user with error messages in the popup ?

@DeepDiver1975
Copy link
Member

Can you remind us what the purpose of the blacklist is ?
There's another ticket where people asked for removing it completely.

This is about client side blacklisting - totally unrelated to our server blacklist.
On the client the blacklist contains files which are not to be synced - e.g. files with unsupported file names or where the server told the client to nolonger sync them because of they violate any kind of pattern (e.g. invalid file size or type - which is used in some enterprise environments)

@guruz
Copy link
Contributor

guruz commented Sep 2, 2014

files with unsupported file names or where the server told the client to nolonger sync them because of they violate any kind of pattern (e.g. invalid file size or type - which is used in some enterprise environments)

The question is if all possible storage backends return a 4xx for those kind of error or a 5xx.
If the latter, then we'll continue to blacklist 5xx but distinguish the 503 (and maybe some others the server guys tell us)

@guruz
Copy link
Contributor

guruz commented Oct 27, 2014

Reminder: Maybe the update phase of csync must also handle the 503 Storage Unvailable differently. Currently it would abort the whole sync.
Question is if we need to do better, why would the storage be unavilable. Sysadmin should fix it :-)

@PVince81
Copy link
Contributor Author

One reason for example is if you connect to a remote ownCloud, SFTP or sharepoint, and the remote password has changed, in which case we also return 503 unavailable.
Or maybe at some point there are connectivity issues.

@PVince81
Copy link
Contributor Author

There are also discussions about providing an offline mode for storages in the future: owncloud/core#11149

@dragotin
Copy link
Contributor

If the server could send the directory for which the 503 is valid, we could set it to ignore... maybe. @PVince81 would that be feasible?

@PVince81
Copy link
Contributor Author

In what way would the it be sent ?

Currently in my test case I have a folder "/oc" which returns 503 when you do any operations on it.
However running PROPFIND on "/" will work fine (after owncloud/core#11791 is merged)

In what part of the discovery should information be returned ?

@dragotin
Copy link
Contributor

This branch proposes a fix for this problem: https://github.com/owncloud/mirall/tree/fix_service_unavailable

@DeepDiver1975
Copy link
Member

please keep in mind that due to e.g. upgrade we will respond with 503 at any time - you cannot rely on the resource type you are asking for.

the clean solution would be to have dedicated status codes for dedicated situations

@ogoffart
Copy link
Contributor

well, 503 on status.php will abort the sync anyway.

@DeepDiver1975
Copy link
Member

well, 503 on status.php will abort the sync anyway.

sure - but it can happen during any other sync step as well and you can make no assumption about the reason

@guruz guruz added this to the 1.9 - Multi-account milestone Feb 7, 2015
@guruz guruz self-assigned this Feb 7, 2015
@guruz guruz assigned ckamm and unassigned guruz Feb 25, 2015
@ckamm
Copy link
Contributor

ckamm commented Feb 25, 2015

I'm closing this ticket because we handle

  • 503 Storage not available during discovery and
  • regular 503 during connection pings

correctly now.

A upgrade-induced 503 during discovery or up/downloads will lead to a sync failure. I'll make a new ticket to track and discuss that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants