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

users/config: Add a warning to ignoreDelete #608

Merged
merged 3 commits into from Feb 12, 2021

Conversation

tomasz1986
Copy link
Contributor

Add a warning similar to disableFsync clearly stating that this option
should be used at one's own risk.

Signed-off-by: Tomasz Wilczyński twilczynski@naver.com

Add a warning similar to disableFsync clearly stating that this option
should be used at one's own risk.

Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
@imsodin
Copy link
Member

imsodin commented Feb 12, 2021

I'd add something explicitly saying "not recommended to use" or just replace the entire description with a link to https://docs.syncthing.net/advanced/folder-ignoredelete.html, which already has a warning.

Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
@tomasz1986
Copy link
Contributor Author

Done. I think that having the warning here is useful though, so that people can see it immediately and hopefully get discouraged from using it without having to go deeper into the Docs for a more detailed explanation.

@imsodin imsodin merged commit ec53728 into syncthing:main Feb 12, 2021
@tomasz1986 tomasz1986 deleted the tomasz86-ignoredelete branch February 12, 2021 16:51
@AudriusButkevicius
Copy link
Member

Tbh, I would have made it more scary, saying this will corrupt the database and lead to permanently out of sync status etc.

@tomasz1986
Copy link
Contributor Author

tomasz1986 commented Feb 12, 2021

Tbh, I would have made it more scary, saying this will corrupt the database and lead to permanently out of sync status etc.

I can add it, but would it not be too long for the general config page?

If yes, then it could definitely fit nicely at https://docs.syncthing.net/advanced/folder-ignoredelete.html instead.

@@ -331,6 +331,9 @@ order
the 1 KB becomes known to the pulling device.

ignoreDelete
.. warning::
Enabling this is highly not recommended - use at your own risk.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling this will willingly corrupt the database and lead to permanently out of sync state - use at your own risk

tomasz1986 added a commit to tomasz1986/docs that referenced this pull request Feb 12, 2021
…cthing#608)

Make the warning even scarier in order to discourage users from even
thinking about enabling it.

Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
@imsodin
Copy link
Member

imsodin commented Feb 12, 2021

I am all for making it super scary - however where exactly does db corruption come into this? I am only aware of making the device out-of-sync forever, not of any db corruption.

@AudriusButkevicius
Copy link
Member

It's corrupt in terms of state of the world as you are missing delete entries, and potentially have gaps in the sequences?

@imsodin
Copy link
Member

imsodin commented Feb 12, 2021

You have the full view of the world, you just choose to ignore the deletions when pulling. Thus leaving you out-of-sync on those items compared to the world.

@AudriusButkevicius
Copy link
Member

Huh, I went looking for the code in the protocol package where it used drop the fileinfos before they even reached the model, and to my surprise it's no longer there.

It seems the deletes are not dropped anymore, they are just ignored in the Need part, which I assume means that if you disable the flag, things should go back to normal?

@imsodin
Copy link
Member

imsodin commented Feb 12, 2021

Basically yes.

@tomasz1986
Copy link
Contributor Author

This information does make the new PR (#611) kind of irrelevant, right? I mean, if the database is not getting corrupted, and the state is reversible, then ultimately what I wrote there is simply not true 😏.

@AudriusButkevicius
Copy link
Member

Agree.

calmh added a commit that referenced this pull request Mar 9, 2021
* main: (21 commits)
  Remove dead Badger experiment
  users/config: Describe QUIC address config (#618)
  Mention QUIC related port (#617)
  dev/release-creation: Adjust to changes to grt tool
  dev/building: Update go version to 1.16 (#614)
  dev/building: Add Windows and Android prerequisite information (#615)
  Update versioning.rst (#613)
  users/config: Add a warning to ignoreDelete (#608)
  users/versioning: Improve external versioning example script for Windows (#606)
  gui: Make terms bold in definition lists for better visibility (#609)
  dev/rest: Update db/browse example
  dev/release-creation: Create new milestone
  users/config: Use backslashes for Windows paths (#603)
  users/config: Improve modTimeWindowS explanation (#584)
  users/foldertypes: Button wording (#602)
  events: Add PendingDevicesChanged and PendingFoldersChanged (ref #592) (#597)
  users/config: Document decoupled database + config (fixes #571) (#600)
  users/stdiscosrv, users/strelaysrv: Better documentation how to obtain the binaries. (#601)
  advanced/folder-ignoredelete: Add a warning about using ignoreDelete (#598)
  rest: API for pending devices and folders. (#498)
  ...
@syncthing syncthing locked and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants