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 all GC TTL and range size mentions to DRY #16506

Merged
merged 1 commit into from
Mar 29, 2023
Merged

Conversation

rmloveland
Copy link
Contributor

... where DRY = Don't Repeat Yourself

Addresses:

DOC-6539
DOC-6540
DOC-6820
DOC-6834

Summary of changes:

  • Update all mentions in v23.1 docs of the default GC TTL (gc.ttlseconds) and default/max range size (range_max_bytes) to point to their respective entries on the 'Configure Replication Zones' page.

  • Update the description of the gc.ttlseconds zcfg variable to link to more salient info re: MVCC, range splits, etc. to provide the reader with add'l context

  • Make a few small text updates to weave in the above, as well as a few opportunistic link additions (to range splitting docs, for example)

@netlify
Copy link

netlify bot commented Mar 16, 2023

Netlify Preview

Name Link
🔨 Latest commit 36f3e93
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/64230a5886e6210007c1f329
😎 Deploy Preview https://deploy-preview-16506--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rmloveland
Copy link
Contributor Author

@irfansharif added you since my understanding is you are the primary engineer on the change to the GC TTL default for v23.1 based on cockroachdb/cockroach#89233, cockroachdb/cockroach#93836, and your filing of cockroachdb/cockroach#98105

@kathancox adding you as well per our discussion earlier today. I suspect some of these changes will be obsoleted by your work on DOC-1012, DOC-6654, and friends, but hopefully this PR is not too disruptive b/c the changes are small and will ensure consistent pointing to one place for the GC TTL default values (even if for a short time because your subsequent work will be rewriting some of this stuff Very Soon (tm))

@rmloveland
Copy link
Contributor Author

also @irfansharif there will almost certainly be more doc updates to come re: the new GC TTL

this is a first pass at just pointing users to one place for the updated defaults and making sure when they get to that one place we have more cross-linking to concepts like MVCC and range splits they need to understand if they are going to be changing this setting

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for setting the bed for the forthcoming GC TTL specific documentation. It LGTM, but I noticed some out-of-date or incorrect information that I've commented on below. Those corrections are unrelated to what this PR itself is changing, but appeared adjacent to text this PR changed. So it caught my eye.

v23.1/delete-data.md Outdated Show resolved Hide resolved
v23.1/backup.md Outdated Show resolved Hide resolved
_includes/v23.1/zone-configs/variables.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kathancox kathancox left a comment

Choose a reason for hiding this comment

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

From a docs pov, LGTM! I added a potentially helpful link re Irfan's comment.

_includes/v23.1/zone-configs/variables.md Outdated Show resolved Hide resolved
@rmloveland
Copy link
Contributor Author

Thanks for the reviews @irfansharif and @kathancox ! I have made updates based on your comments. Please let me know if I got them approximately right, or if further changes are needed

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

LGTM.

... where DRY = Don't Repeat Yourself

Addresses:

DOC-6539
DOC-6540
DOC-6820
DOC-6834

Summary of changes:

- Update all mentions in v23.1 docs of the default GC
  TTL (`gc.ttlseconds`) and default/max range size (`range_max_bytes`)
  to point to their respective entries on the 'Configure Replication
  Zones' page.

- Update the description of the `gc.ttlseconds` zcfg variable to link to
  more salient info re: MVCC, range splits, etc. to provide the reader
  with add'l context

- Make a few small text updates to weave in the above, as well as a few
  opportunistic link additions (to range splitting docs, for example)

- Update several places that make it seem like GC runs at a specific
  time (whether 25 hours or, now, 4 hours) to hopefully make it clear
  that things are *eligible* for GC past a threshold

- Update `gc.ttlseconds` replication zone variable docs to make it
  clearer that the recommendations around GC TTL vs backups only apply
  if you are not using scheduled backups (which you should be)
Copy link
Contributor

@ianjevans ianjevans left a comment

Choose a reason for hiding this comment

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

LGTM!

@rmloveland rmloveland merged commit d486cce into master Mar 29, 2023
@rmloveland rmloveland deleted the 20230316-DOC-6820 branch March 29, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants