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

Update searchable snapshot to clarify #108750

Merged
merged 5 commits into from
May 17, 2024
Merged

Conversation

kunisen
Copy link
Contributor

@kunisen kunisen commented May 17, 2024

  • Have you signed the contributor license agreement? Yes
  • Have you followed the contributor guidelines? Yes
  • If submitting code, have you built your formula locally prior to submission with gradle check? Not code
  • If submitting code, is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed. Not code
  • If submitting code, have you checked that your submission is for an OS and architecture that we support? Not code
  • If you are submitting this code for a class then read our policy for that. Not code

Update searchable snapshot to clarify it's not a backup of index data and deleting it will result in permanent data loss.

It's a follow up of #108451

Update searchable snapshot to clarify it's not a backup of index data and deleting it will result in permanent data loss. 

It's a follow up of #108451
@kunisen kunisen added >docs General docs changes :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Supportability Improve our (devs, SREs, support eng, users) ability to troubleshoot/self-service product better. labels May 17, 2024
Copy link

Documentation preview:

@elasticsearchmachine
Copy link
Collaborator

@kunisen please enable the option "Allow edits and access to secrets by maintainers" on your PR. For more information, see the documentation.

@elasticsearchmachine elasticsearchmachine added v8.15.0 Team:Distributed Meta label for distributed team Team:Docs Meta label for docs team external-contributor Pull request authored by a developer outside the Elasticsearch team labels May 17, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Member

@geekpete geekpete left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

IME it doesn't work very well to start sections with WARNING admonitions like this. Maybe one is ok, but then we add another, and another, and it ends up pushing the actual content of the section out of view.

Also, this warning is kind of confusing (what does it have to do with frozen/cold data?) and also kind of ambiguous (deleting the searchable snapshot index does not cause permanent data loss, it's only deleting the underlying snapshot while it's still mounted that causes problems).

Instead, maybe we should make this whole section into a WARNING, and add some more detail to each of the bullet points in the list about the consequences of ignoring the advice.

@kunisen
Copy link
Contributor Author

kunisen commented May 17, 2024

Thank you @DaveCTurner so much for the quick and detailed guidance!
I didn't want to break the whole text so I only added a piece of warning, but indeed as you said, it's good to make the whole section a warning to bring the best awareness.

We are good with this change. If you don't mind, may I also ask your help to see if we can merge this PR, or if you can kindly forward this to doc team for doc-related reviews? 🙏

Thanks again!

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner merged commit e2f9f64 into main May 17, 2024
6 checks passed
@DaveCTurner DaveCTurner deleted the kunisen-docpr-followUpOn108451 branch May 17, 2024 12:40
@kunisen
Copy link
Contributor Author

kunisen commented May 17, 2024

thanks!
sorry forgot to add backport label.

DaveCTurner added a commit that referenced this pull request May 17, 2024
Relates #108451

Co-authored-by: David Turner <david.turner@elastic.co>
DaveCTurner added a commit that referenced this pull request May 17, 2024
Relates #108451

Co-authored-by: David Turner <david.turner@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >docs General docs changes external-contributor Pull request authored by a developer outside the Elasticsearch team Supportability Improve our (devs, SREs, support eng, users) ability to troubleshoot/self-service product better. Team:Distributed Meta label for distributed team Team:Docs Meta label for docs team v8.13.5 v8.14.1 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants