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

wincfapi: database dotfiles have 'syncing' arrows #7141

Closed
jnweiger opened this issue Apr 12, 2019 · 5 comments
Closed

wincfapi: database dotfiles have 'syncing' arrows #7141

jnweiger opened this issue Apr 12, 2019 · 5 comments
Assignees
@ckamm @HanaGemela
Labels
feature:vfs native virtual files and placeholder implementation ReadyToTest QA, please validate the fix/enhancement type:bug
Milestone
2.6.0

Comments

@jnweiger
Copy link
Contributor

jnweiger commented Apr 12, 2019 •
edited
Loading

tested ownCloud-2.6.0.11775.11588-daily20190411.GPO.msi with vfs enabled on win10

grafik

The client should not try to sync the database files.
@ckamm ckamm assigned ckamm and unassigned guruz Apr 15, 2019
@ckamm ckamm added type:bug feature:vfs native virtual files and placeholder implementation labels Apr 15, 2019
@ckamm ckamm added this to the 2.6.0 milestone Apr 15, 2019
@ckamm
Copy link
Contributor

ckamm commented Apr 15, 2019

Ah, the problem is that to remove the "needs syncing" icon the client needs to explicitly set an "excluded" state on the files. It does that, but it looks like the files are recreated on sync, wiping that flag again.

ckamm added a commit that referenced this issue Apr 16, 2019
@ckamm
SyncEngine: Don't close db when done #7141
3b58794 
The db-close operation is likely a leftover from when the SyncEngine
owned its own db connection and serves no purpose anymore.

Closing the db causes the removal of the temporary wal and shm files.
These files are recreated when the db is opened again, which happens
almost immediately.

This is a problem for winvfs because the delete-recreate step wipes the
exclusion state on these files just after the sync is done. That meant
that the db temporaries permanently had a "needs sync" icon marker shown
in the explorer.

Avoiding reopening the db also reduces the number of log messages per
sync.
@ckamm ckamm mentioned this issue Apr 16, 2019
Merged
ckamm added a commit that referenced this issue Apr 16, 2019
@ckamm
SyncEngine: Don't close db when done #7141
2a90090 
The db-close operation is likely a leftover from when the SyncEngine
owned its own db connection and serves no purpose anymore.

Closing the db causes the removal of the temporary wal and shm files.
These files are recreated when the db is opened again, which happens
almost immediately.

This is a problem for winvfs because the delete-recreate step wipes the
exclusion state on these files just after the sync is done. That meant
that the db temporaries permanently had a "needs sync" icon marker shown
in the explorer.

Avoiding reopening the db also reduces the number of log messages per
sync.
@ckamm ckamm added ReadyToTest QA, please validate the fix/enhancement and removed PR available labels Apr 16, 2019
@HanaGemela HanaGemela self-assigned this Apr 18, 2019
@HanaGemela
Copy link
Contributor

HanaGemela commented Apr 23, 2019 •
edited
Loading

"Needs syncing" icon is visible in 2.6.0alpha1 (build 11573) after restart

Steps to recreate:

  1. Stop the server
  2. Quit ownCloud client
  3. Start ownCloud client
  4. Check the "needs syncing" icons

Actual result: The icons are shown until the client is connected to the server. Then the icons disappear.
Expected result: The client should not try to sync the database files.

@HanaGemela HanaGemela removed the ReadyToTest QA, please validate the fix/enhancement label Apr 23, 2019
@ckamm
Copy link
Contributor

ckamm commented Apr 23, 2019

@HanaGemela Thanks for the test, I see the problem - my fix only addresses things after the first sync has completed.

ckamm added a commit that referenced this issue Apr 23, 2019
@ckamm
Vfs: Mark sqlite temporaries excluded on db-open #7141
9271eb5 
The previous patch ensured that the sqlite temporaries weren't deleted
and recreated for every sync run, but there was still time between
client startup and the first sync run where they would have the
"needs-sync" icon.
@ckamm ckamm mentioned this issue Apr 23, 2019
Merged
@ckamm
Copy link
Contributor

ckamm commented Apr 23, 2019 •
edited
Loading

With the new patch the icon does not appear or appears only very briefly even if the sync folder is paused on startup.

ckamm added a commit that referenced this issue Apr 26, 2019
@ckamm
Vfs: Mark sqlite temporaries excluded on db-open #7141
03b65e2 
The previous patch ensured that the sqlite temporaries weren't deleted
and recreated for every sync run, but there was still time between
client startup and the first sync run where they would have the
"needs-sync" icon.
@ckamm ckamm added ReadyToTest QA, please validate the fix/enhancement and removed PR available labels Apr 26, 2019
@HanaGemela
Copy link
Contributor

Tested

Tested on windows with 2.6.0daily20190429 (build 11644)
Syncing arrows don't appear against the files mentioned above

ckamm added a commit that referenced this issue Jun 5, 2019
@ckamm
SyncEngine: Don't close db when done #7141
d40ee04 
The db-close operation is likely a leftover from when the SyncEngine
owned its own db connection and serves no purpose anymore.

Closing the db causes the removal of the temporary wal and shm files.
These files are recreated when the db is opened again, which happens
almost immediately.

This is a problem for winvfs because the delete-recreate step wipes the
exclusion state on these files just after the sync is done. That meant
that the db temporaries permanently had a "needs sync" icon marker shown
in the explorer.

Avoiding reopening the db also reduces the number of log messages per
sync.

(cherry picked from commit 2a90090)

Cherry picked as it seems likely to decrease the severety of #6877: if
the database isn't closed and reopened as frequently users are less
likely to run into crashes when the sqlite connection switches to wal
journaling mode.
ckamm added a commit that referenced this issue Jun 7, 2019
@ckamm
SyncEngine: Don't close db when done #7141
d7d5586 
The db-close operation is likely a leftover from when the SyncEngine
owned its own db connection and serves no purpose anymore.

Closing the db causes the removal of the temporary wal and shm files.
These files are recreated when the db is opened again, which happens
almost immediately.

This is a problem for winvfs because the delete-recreate step wipes the
exclusion state on these files just after the sync is done. That meant
that the db temporaries permanently had a "needs sync" icon marker shown
in the explorer.

Avoiding reopening the db also reduces the number of log messages per
sync.

(cherry picked from commit 2a90090)

Cherry picked as it seems likely to decrease the severety of #6877: if
the database isn't closed and reopened as frequently users are less
likely to run into crashes when the sqlite connection switches to wal
journaling mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ckamm ckamm

@HanaGemela HanaGemela

Labels
feature:vfs native virtual files and placeholder implementation ReadyToTest QA, please validate the fix/enhancement type:bug
Projects
None yet
Milestone
2.6.0
Development

No branches or pull requests
4 participants
@ckamm @guruz @jnweiger @HanaGemela