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

Retention doc #3775

Merged
merged 7 commits into from
Jun 7, 2021
Merged

Retention doc #3775

merged 7 commits into from
Jun 7, 2021

Conversation

cyriltovena
Copy link
Contributor

This adds documentation for the experimental custom retention feature through the compactor.

/cc @sandeepsukhani see how you can hook up your deletion documentation into it.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
@KMiller-Grafana
Copy link
Contributor

As a more general question, if this new documentation describes an experimental feature, should there be prose that explicitly states which parts are experimental?

@cyriltovena
Copy link
Contributor Author

As a more general question, if this new documentation describes an experimental feature, should there be prose that explicitly states which parts are experimental?

Totally agree, this is stated line 12. Do you think it should be more explicit than that ?

The Compactor retention will become the default and have long term support. While this retention is still experimental, it supports more granular retention on per tenant and per stream use cases.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Copy link
Contributor

@KMiller-Grafana KMiller-Grafana left a comment

Choose a reason for hiding this comment

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

A few last corrections I would like you to consider. I believe the prose is good once these changes are made.

@@ -5,33 +5,31 @@ title: Retention

Retention in Loki is achieved either through the [Table Manager](#table-manager) or the [Compactor](#Compactor).

## Which one should you use ?
Which one should you use ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line completely. We don't need the question, since the sentence above it implies that we'll be describing both.

docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved

2. It gives you a short period to cancel chunks deletion in case of mistakes.

Marker files (containing chunks to deletes), should be store on a persistent disk, since this is the sole reference to them anymore.
Copy link
Contributor

Choose a reason for hiding this comment

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

For now, let's fix some grammar, and consider in the future how to make this better. In the future, we should either explain how to cancel the deletion or provide a link within the docs to the place where it is explained.

docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
docs/sources/operations/storage/retention.md Outdated Show resolved Hide resolved
cyriltovena and others added 4 commits June 7, 2021 09:02
Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>
Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>
Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>
Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>
@cyriltovena cyriltovena merged commit 71677db into grafana:main Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants