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

Handle errors of database transactions #4632

Closed
dragotin opened this issue Apr 1, 2016 · 8 comments
Closed

Handle errors of database transactions #4632

dragotin opened this issue Apr 1, 2016 · 8 comments
Assignees
Milestone

Comments

@dragotin
Copy link
Contributor

dragotin commented Apr 1, 2016

Expected behaviour

It can happen that even if the sync journal was opened successfully, that subsequent interactions fail. For that we need to check the return value of low level SyncJournal methods like setFileRecord and others.

If for example setFileRecord fails, we need to bail out with a critical error.

Actual behaviour

The return values are widely not checked because we assumed that if the db was opened successfully all is good.

Version where we found that: 2.1.1 and 2.2.0pre

@dragotin dragotin added the bug label Apr 1, 2016
@dragotin dragotin added this to the 2.2.0-current milestone Apr 1, 2016
@guruz
Copy link
Contributor

guruz commented Apr 3, 2016

Also make sure the asserts are real problems, not a problem in debug mode for 5 developers.
#3906 (comment)

@dragotin
Copy link
Contributor Author

dragotin commented Apr 8, 2016

After some more debugging we found out the root cause: The user is syncing to a removable disk. Later the user hibernates the machine and removes the removable disk. When he starts the computer again, the disk is still not connected, he does it seconds later once the machine is up and running again.

If the sync is running when the machine is hibernated, especially the database file is open. If it wakes up from hibernation but the external disk is not yet connected, the database file is lost for the sync process, and that is why sql commands fails.

In that case the client needs to bail out with an error and restart the sync.

ogoffart added a commit that referenced this issue Apr 11, 2016
ogoffart added a commit that referenced this issue Apr 11, 2016
@dragotin
Copy link
Contributor Author

Some more pull requests to this:

Setting this to ready to test.

@owncloud/qa you find testing instructions here
@msrex for your info, please try nightly builds starting tomorrow.

@dragotin dragotin added the ReadyToTest QA, please validate the fix/enhancement label Apr 11, 2016
@mcastroSG
Copy link

mcastroSG commented May 5, 2016

Hi Hi:

TEST1

Steps Executed

  1. Add a new account setting as Sync Shared Folder a USB
  2. Pause Sync
  3. Copy a big amount of data to the USB
  4. Restart Sync
  5. Pause Sync
  6. Hibernate PC
  7. Unplug USB
  8. Come out of hibernation

Results

An error related to the db fail is shown. USB gets corrupted during the process so we need to review and repair it with a tool included at W10. After that just Unpause Sync and a sync starts without problem.

img_05052016_090129

At snapshot attached appears the pause sync icon due to we tried to pause sync before hibernate but it does not work in this way, sync is atomic and in case we try to pause during sync it continues.

##TEST2

Steps Executed

  1. Add a new account setting as Sync Shared Folder a USB
  2. Copy a big amount of data to the USB
  3. While Sync Hibernate PC
  4. Unplug USB
  5. Come out of hibernation

Results

There were no error related to DB, just an error that informs about there is no Shared Folder mounted. It seems that detects there is no USB mounted and shows just that error. After that and plug in again the USB it began to work.

@dragotin could we consider these results as valid ?

@mcastroSG
Copy link

@msrex your input is appreciated too, are the results ok?

@dragotin
Copy link
Contributor Author

dragotin commented May 9, 2016

I think these tests are fine. The important bits are:

  • There is a error message about the problem (happens as we see)
  • once the problem is resolved but repair or remount, the sync continues without up- or download or delete all the files again.

@rperezb
Copy link

rperezb commented May 9, 2016

great, @mcastroSG time for us to close this?

@mcastroSG
Copy link

sure 😄

@mcastroSG mcastroSG removed the ReadyToTest QA, please validate the fix/enhancement label May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants