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 Row-Level TTL docs to note full support #15358

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

rmloveland
Copy link
Contributor

Fixes DOC-4756

Summary of changes:

  • Update 'Batch Delete Data with Row-Level TTL' page to note:

    • You can now do per-row TTL using ttl_expiration_expression, so we mention that and give examples, describe how it's validated

    • Remove at least the resolved limitations from the TTL docs and the 'Known Limitations' page

    • Remove uses of ttl_automatic_column which is no longer used in v22.2+

  • Update 'WITH storage_parameter' page to use new output / TTL params

  • Update 'RESET storage_parameter' page to use new output / TTL params

  • Update included table of storage parameters to remove duplication of TTL params, instead linking to the canonical location on the TTL docs page

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Oct 13, 2022

Files changed:

@netlify
Copy link

netlify bot commented Oct 13, 2022

Netlify Preview

Name Link
🔨 Latest commit 5529bc1
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/637276c6450deb00084c37b7
😎 Deploy Preview https://deploy-preview-15358--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 rmloveland force-pushed the 20221011-full-support-row-level-ttl branch from 8cfdd63 to 3efa2c8 Compare October 13, 2022 19:50
@rmloveland rmloveland requested review from ecwall and rafiss October 14, 2022 16:15
@rmloveland
Copy link
Contributor Author

@rafiss added you as reviewer alongside @ecwall b/c of cockroachdb/cockroach#86269

let me know if you would rather not be added tho!

Pretty sure I caught most of the changes from reading PRs and playing with the DB but ... entirely possible I missed something. Please let me know if so. Also happy to make any other updates re: clarity, etc. Thanks!

@rafiss
Copy link
Contributor

rafiss commented Oct 14, 2022

thanks @rmloveland ! two points before doing a deeper review:

  • @ecwall : are you planning to separate out the metrics name change in ttl: Remove ttl_range_concurrency config cockroach#89392 and backport that to v22.2.0? (I think we should.) If so, then we can document that here too.
  • I noticed that we don't document the TTL metrics dashboard in the DB Console currently. I think we should add docs for that now.

@ecwall
Copy link

ecwall commented Oct 14, 2022

thanks @rmloveland ! two points before doing a deeper review:

  • @ecwall : are you planning to separate out the metrics name change in ttl: Remove ttl_range_concurrency config cockroach#89392 and backport that to v22.2.0? (I think we should.) If so, then we can document that here too.
  • I noticed that we don't document the TTL metrics dashboard in the DB Console currently. I think we should add docs for that now.

Yeah it looks like some of the metrics are not working properly in the db console so I’m fixing that also.

@rmloveland rmloveland force-pushed the 20221011-full-support-row-level-ttl branch from 3efa2c8 to 1931aa8 Compare October 18, 2022 20:06
@rmloveland
Copy link
Contributor Author

rmloveland commented Oct 18, 2022 via email

@rmloveland
Copy link
Contributor Author

rmloveland commented Oct 18, 2022 via email

- <a name="ttl-cannot-be-customized"></a> The TTL cannot be customized based on the values of other columns in the row. [cockroachdb/cockroach#76916](https://github.com/cockroachdb/cockroach/issues/76916)
- Because of the above limitation, adding TTL to large existing tables [can negatively affect performance](row-level-ttl.html#ttl-existing-table-performance-note), since a new column must be created and backfilled for every row. Creating a new table with a TTL is not affected by this limitation.
- The queries executed by Row-Level TTL are not yet optimized for performance:
- They do not use any indexes that may be available on the [`crdb_internal_expiration` column](row-level-ttl.html#crdb-internal-expiration).
Copy link

Choose a reason for hiding this comment

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

This is still a limitation, but I would expand it to say that it does not use any indexes (not just indexes on crdb_internal_expiration).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated - PTAL!

|---------------------+----------------------+-----+------|
| `exclude_data_from_backup` | Excludes the data in this table from any future backups. | Boolean | `false` |
| `ttl` | Signifies if a TTL is active. Automatically set and controls the reset of all TTL-related storage parameters. | N/A | N/A |
| `ttl_automatic_column` | If set, use the value of the `crdb_internal_expiration` hidden column. Always set to `true` and cannot be reset. | Boolean | `true` |
Copy link

Choose a reason for hiding this comment

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

All of these are still used except for ttl_automatic_column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes definitely! All of the TTL params are now moved to row-level-ttl.md (line 128, the TTL Storage Parameters section) so all the TTL things are documented in one place

| `ttl_job_cron` | The frequency at which the TTL job runs. | [CRON syntax](https://cron.help) | `'@hourly'` |
| `ttl_label_metrics` | Whether or not [TTL metrics](row-level-ttl.html#ttl-metrics) are labelled by table name (at the risk of added cardinality). | Boolean | `false` |
| `ttl_pause` | If set, stops the TTL job from executing. | Boolean | `false` |
| `ttl_range_concurrency` | The Row-Level TTL queries split up scans by ranges, and this determines how many concurrent ranges are processed at a time. Minimum: `1`. | Integer | `1` |
Copy link

Choose a reason for hiding this comment

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

This is used in 22.2.0, but not 22.2.1+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked and ttl_range_concurrency is not in the list of params on row-level-ttl.md (line 128, the TTL Storage Parameters section)

- [Per-table TTL](#per-table-ttl)
- [Per-row TTL](#per-row-ttl)

### Per-table TTL
Copy link

Choose a reason for hiding this comment

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

The difference between ttl_expire_after and ttl_expiration_expression isn't per-table vs per-row. Both of them set TTL per-row in the table.

ttl_expire_after is a convenient way of setting rows to expire a fixed amount of time after they are created or updated.

ttl_expiration_expression lets you customize the expiration logic further by providing an expression. (You could get the same behavior as ttl_expire_after by creating a column with a default value and having the ttl_expiration_expression reference that column.)

Additionally, the 2 storage params can co-exist where the ttl_expire_after creates the crdb_internal_expiration column and ttl_expiration_expression overrides the default value of crdb_internal_expiration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying this! I have updated these sections of the doc to add this info - PTAL

---------------------------------------+-------------+----------------------------+--------------------------------
a9404386-c4da-415f-b0b0-0dfad0f13c80 | a thing | 2022-04-19 19:53:00.037216 | 2023-04-19 19:57:27.592898+00
(1 row)
ALTER TABLE events RESET (ttl);
Copy link

Choose a reason for hiding this comment

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

You can also do ALTER TABLE events RESET (ttl_expire_after); or ALTER TABLE events RESET (ttl_expiration_expression); if both are set and you want to remove one or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this info - PTAL

@rafiss rafiss requested a review from ecwall November 7, 2022 21:17
@rmloveland rmloveland force-pushed the 20221011-full-support-row-level-ttl branch from 1931aa8 to d295707 Compare November 8, 2022 20:16
@rmloveland
Copy link
Contributor Author

per convo with @dikshant : I will update this PR to state that ttl_expiration_expression is preferred over ttl_expire_after starting with v22.2 for the following reasons:

  • if you add ttl_expire_after it will cause a full table rewrite. you can't use this with an existing timestamp column
  • whereas ttl_expiration_expression can say "please expire based on this updated_at col that already exists" etc

Fixes:

- DOC-4664
- DOC-4672
- DOC-4756
- DOC-4881
- DOC-5270
- DOC-5983
- DOC-5995

Summary of changes:

- Update 'Batch Delete Data with Row-Level TTL' page to note:

  - You can now do per-row TTL using `ttl_expiration_expression`, so we
    mention that and give examples, describe how it's validated

  - Remove at least the resolved limitations from the TTL docs and the
    'Known Limitations' page

  - Remove uses of `ttl_automatic_column' which is no longer used in
    v22.2+

  - Remove `ttl_range_concurrency`, which is no longer available in
    v22.2+ per cockroachdb/cockroach#89392

  - Add note on migrating from versions < v22.2

  - Add recommendation that most users use `ttl_expiration_expression`
    over `ttl_expire_after`

    - Including moving sections of the page to put the expression info
      first

- Add page for TTL Dashboard in DB Console docs

- Update 'WITH storage_parameter' page to use new output / TTL params

- Update 'RESET storage_parameter' page to use new output / TTL params

- Update included table of storage parameters to remove duplication of
  TTL params, instead linking to the canonical location on the TTL docs page
@rmloveland rmloveland force-pushed the 20221011-full-support-row-level-ttl branch from d295707 to 5529bc1 Compare November 14, 2022 17:11
@rmloveland
Copy link
Contributor Author

@rafiss and @ecwall this has been updated with your feedback and the comment from @dikshant and should be RFAL

Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

nice, lgtm!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ecwall)

@rmloveland rmloveland requested a review from ianjevans November 21, 2022 18:44
@rmloveland rmloveland merged commit fd0b52c into master Nov 28, 2022
@rmloveland rmloveland deleted the 20221011-full-support-row-level-ttl branch November 28, 2022 20:59
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.

5 participants