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

23.1 GA Release Notes #16951

Merged
merged 19 commits into from
May 15, 2023
Merged

23.1 GA Release Notes #16951

merged 19 commits into from
May 15, 2023

Conversation

mikeCRL
Copy link
Contributor

@mikeCRL mikeCRL commented May 12, 2023

To Do:

  • Ensure everything that should be is labeled Enterprise (maybe just Changefeeds; if appropriate, I think the whole section can just have a single mention above the table)
  • Add content around cluster settings (intro to Key Changes list) (context/default handling).
  • (DONE, will push with sufficient internet connection soon) Marketing-related copy for this page's intro: Mike awaiting DM from Meagan G as to whether launch page URL is right and if there is any additional Marketing-related copy/linking needed.
  • (DONE, will push with sufficient internet connection soon) Replace various doc link URL formats (eg netlify domain or dev path) with relative docs URLs that use v23.1 path.
  • Add additional doc links
    • @kathancox: Please comment with CDC (just Parquet needed) and DR links needed.
    • @ianjevans: Please add asyncpg.
  • Clarify what extra step is needed in this PR per the wiki, where it states “in the same PR, remove the redirects from alpha release pages added to prevent broken links in deprecation error messages.” (@nickvigilante do you know what this is?)
  • (DONE, will push with sufficient internet connection soon) Confirm how to compile Deprecations and move/copy backward-incompatible item show_ranges_deprecated_behavior there.
  • (DONE) Update the Additional Resources section with new docs and training links. (@shannonbradshaw Can you please refresh the current section with some new links e.g. some of the recent team highlights, and anything from Training?)
  • Some final PM reviews.
    • Liv
    • Kevin (minor review, plus need doc link suggestions)
  • (DONE, will push with sufficient internet connection soon) Swap <strong> to <p>
  • Change versions.csv date for 23.1 from 2023-05-10 (required for site preview to work) to 2023-05-15 prior to merging.

Post-release follow-up:

  • Improve cluster settings page to cover [default handling].

@mikeCRL mikeCRL requested a review from a team as a code owner May 12, 2023 06:12
@mikeCRL mikeCRL marked this pull request as draft May 12, 2023 06:12
@github-actions
Copy link

github-actions bot commented May 12, 2023

Files changed:

@netlify
Copy link

netlify bot commented May 12, 2023

Netlify Preview

Name Link
🔨 Latest commit a0f338e
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/645ddb0dc622420008990127
😎 Deploy Preview https://deploy-preview-16951--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.

@netlify
Copy link

netlify bot commented May 12, 2023

Netlify Preview

Name Link
🔨 Latest commit a992949
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/646287ad762c210008784067
😎 Deploy Preview https://deploy-preview-16951--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.

@nickvigilante
Copy link
Contributor

@mikeCRL I think it's just to make sure there are no references directly to v22.2 or {{ site.versions["stable"] }} when it should explicitly be v23.1. I've fixed that.

I also went through and tweaked some of the includes/Liquid to get the docs to build and pass htmltest. Sending a commit for this soon.

@mikeCRL
Copy link
Contributor Author

mikeCRL commented May 12, 2023

@nickvigilante While I updated the tables to use <thead> and <tbody>, I found that the text looked much better by keeping <strong> for the Summaries (and adding a small scss tweak to it to reduce line-height) and using <p> for the Descriptions. Do you think that's OK for now and we can come back to table styles later?

@nickvigilante
Copy link
Contributor

@mikeCRL The fact that the body rows have a larger font than the header rows is weird to me. Is there any reason why the body rows have to have a larger font size? Take a look at the v22.2.0 RN table.


- Replaced the `cdc_prev()` [function](../v23.1/functions-and-operators.html) in favor of a `cdc_prev` tuple. This is an incompatible change that may break [changefeeds](../v23.1/change-data-capture-overview.html) that use the previous `cdc_prev()` function. [#85177][#85177]
- [`SHOW RANGES FOR TABLE`](../v23.1/show-ranges.html) now includes rows for all indexes that support the table. Prior to this change, `SHOW RANGES FOR TABLE foo` was an alias for `SHOW RANGES FOR INDEX foo@primary`. This was causing confusion, as it would miss data for secondary indexes. It is still possible to filter to just the primary index using `SHOW RANGES FOR INDEX foo@primary`. The statement output now also includes the index name. [#93545][#93545]
- CockroachDB now supports sharing storage ranges across multiple indexes/tables. As a result, there is no longer a guarantee that there is at most one SQL object (e.g., table/index/sequence/materialized view) per storage range. Therefore, the columns `table_id`, `database_name`, `schema_name`, `table_name` and `index_name` in `crdb_internal.ranges` and `.ranges_no_leases` have become nonsensical: a range cannot be attributed to a single table/index anymore. As a result:
Copy link
Contributor

Choose a reason for hiding this comment

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

This bullet point and the points below need to clarify the relationship with the new cluster setting wrt the deprecated behavior.

NB: even though the default behavior is compatible with v22.2, I still would like this to be mentioned in the backward-incompatible changes, to stimulate customers to adapt their automation

Copy link
Contributor

Choose a reason for hiding this comment

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

This bullet point and the points below need to clarify the relationship with the new cluster setting wrt the deprecated behavior.

I've updated this so that it says

... cannot be attributed to a single table/index anymore. For the v23.1 release, the default behavior of SHOW RANGES is retained, but users should consider setting the cluster setting sql.show_ranges_deprecated_behavior.enabled to false. This will have the following effects that will become the defaults in a future release:

- Instead of `SELECT range_id FROM crdb_internal.ranges WHERE table_name = $1 OR table_id = $2` (variable / unpredictable table name or ID), use: `SELECT range_id FROM [SHOW RANGES FROM CURRENT_CATALOG WITH TABLES] WHERE table_name = $1 OR table_id = $2`
- Instead of `SELECT start_key FROM crdb_internal.ranges WHERE table_name = 'x'`, use: `SELECT raw_start_key FROM [SHOW RANGES FROM TABLE x WITH KEYS]`
- Instead of `SELECT start_key FROM crdb_internal.ranges WHERE table_name = $1 OR table_id = $2` (unpredictable / variable table name or ID), use: `SELECT raw_start_key FROM [SHOW RANGES FROM CURRENT_CATALOG WITH TABLES, KEYS] WHERE table_name = $1 OR table_id = $2` [#93644][#93644]
- The format of the columns `start_key` and `end_key` for `SHOW RANGES FROM DATABASE` and `SHOW RANGES FROM TABLE` have been extended to include which table/index the key belongs to. This is necessary because a range can now contain data from more than one table/index. [#93644][#93644]
Copy link
Contributor

Choose a reason for hiding this comment

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

i believe this is also subject to the new cluster setting

Copy link
Contributor

Choose a reason for hiding this comment

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

- Instead of `SELECT start_key FROM crdb_internal.ranges WHERE table_name = $1 OR table_id = $2` (unpredictable / variable table name or ID), use: `SELECT raw_start_key FROM [SHOW RANGES FROM CURRENT_CATALOG WITH TABLES, KEYS] WHERE table_name = $1 OR table_id = $2` [#93644][#93644]
- The format of the columns `start_key` and `end_key` for `SHOW RANGES FROM DATABASE` and `SHOW RANGES FROM TABLE` have been extended to include which table/index the key belongs to. This is necessary because a range can now contain data from more than one table/index. [#93644][#93644]
- The format of the columns `start_key` and `end_key` for `SHOW RANGE ... FOR ROW` has been changed to be consistent with the output of `SHOW RANGES FROM INDEX`. [#93644][#93644]
- The output of [`SHOW RANGES`](../v23.1/show-ranges.html) no longer includes `range_size`, `range_size_mb`, `lease_holder`, or `lease_holder_localities` by default. This ensures that `SHOW RANGES` remains fast in the common case. Use the new option `WITH DETAILS` to include these columns. [#93644][#93644]
Copy link
Contributor

Choose a reason for hiding this comment

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

i believe this is also subject to the new cluster setting

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other change, I've prepended the clause

When the cluster setting sql.show_ranges_deprecated_behavior.enabled is set to false (recommended in v23.1), ...

- Instead of `SELECT start_key FROM crdb_internal.ranges WHERE table_name = 'x'`, use: `SELECT raw_start_key FROM [SHOW RANGES FROM TABLE x WITH KEYS]`
- Instead of `SELECT start_key FROM crdb_internal.ranges WHERE table_name = $1 OR table_id = $2` (unpredictable / variable table name or ID), use: `SELECT raw_start_key FROM [SHOW RANGES FROM CURRENT_CATALOG WITH TABLES, KEYS] WHERE table_name = $1 OR table_id = $2` [#93644][#93644]
- The format of the columns `start_key` and `end_key` for `SHOW RANGES FROM DATABASE` and `SHOW RANGES FROM TABLE` have been extended to include which table/index the key belongs to. This is necessary because a range can now contain data from more than one table/index. [#93644][#93644]
- The format of the columns `start_key` and `end_key` for `SHOW RANGE ... FOR ROW` has been changed to be consistent with the output of `SHOW RANGES FROM INDEX`. [#93644][#93644]
Copy link
Contributor

Choose a reason for hiding this comment

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

NB: this one is not subject to the new cluster setting

Copy link
Contributor

Choose a reason for hiding this comment

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

I've reordered this so it appears after the things that are subject to the new cluster setting

@mikeCRL
Copy link
Contributor Author

mikeCRL commented May 12, 2023

@mikeCRL The fact that the body rows have a larger font than the header rows is weird to me. Is there any reason why the body rows have to have a larger font size? Take a look at the v22.2.0 RN table.

@nickvigilante I don't have a strong feeling about the header row size, so I used the default, though I do also think that if the header text matched the bolded first column entries, that might also be odd. I find the small table text (e.g. on 22.2) hard to read at default zoom, and also especially without consistent links this time for the left column, there's otherwise nothing to differentiate those summaries; I find the bold text sets it apart and makes it easier to skim, and avoids this table appearing even more dense/text heavy.

@shannonbradshaw Would you like to help settle a style question that has arisen, by comparing the current 23.1 tables to the 22.2 tables, which use our default style? I happened upon different styles here (which I like) because I ended up needing to transform my WIP tables to HTML instead of Markdown.

@shannonbradshaw
Copy link
Contributor

@mikeCRL do you have a specific question on this? I don't want to infer what you're asking.

@shannonbradshaw
Copy link
Contributor

shannonbradshaw commented May 12, 2023

FWIW, I'm not a fan of the bolded feature name in the table. I also think the cluster setting font size is too small. That said, the font size seems to be the same as previous releases.

@nickvigilante
Copy link
Contributor

@mikeCRL Makes sense. We can add an action item for after the release to make styling adjustments to the table CSS.

@mikeCRL
Copy link
Contributor Author

mikeCRL commented May 12, 2023

@shannonbradshaw Sorry, just trying to decide whether to keep both workarounds on size and boldness, or switch to the default in the prior RNs for now. And I appreciate your feedback.

@nickvigilante any objection to dropping the boldness but keeping the size for this one?

@nickvigilante
Copy link
Contributor

@mikeCRL No objections with that.

@mikeCRL
Copy link
Contributor Author

mikeCRL commented May 12, 2023

For others on the PR, this is the exchange that prompted knz's comments above.

@mikeCRL
Copy link
Contributor Author

mikeCRL commented May 12, 2023

Thank you @knz. @rmloveland would you mind reviewing Raphael's comments and making/suggesting any edits to the Backward-Incompatible Changes section bullets on SHOW RANGES and crdb_internal.ranges?

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.

A couple tweaks to DR/CDC and one row removed.

</td>
</tr>
<tr>
<td><p>Add parquet format to changefeeds </p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Parquet format for changefeeds needs to be removed, we don't want this to be public yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a known limitation I added for Parquet (https://github.com/cockroachdb/docs/pull/16902/files) - should I remove?

_includes/releases/v23.1/v23.1.0.md Outdated Show resolved Hide resolved
_includes/releases/v23.1/v23.1.0.md Outdated Show resolved Hide resolved
_includes/releases/v23.1/v23.1.0.md Outdated Show resolved Hide resolved
<tr>
<td><p>Enforce supported backup versions</p>
</td>
<td><p>To help ensure backups and restores are successful, CockroachDB now enforces its previous support for restoring backups from up to two minor versions prior. Previously, restoring backups produced from even earlier versions was possible, but unreliable. Now, this operation is prevented with an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<td><p>To help ensure backups and restores are successful, CockroachDB now enforces its previous support for restoring backups from up to two minor versions prior. Previously, restoring backups produced from even earlier versions was possible, but unreliable. Now, this operation is prevented with an error.
<td><p>To help ensure backups and restores are successful, CockroachDB now enforces its previous [support for restoring backups](restoring-backups-across-versions.html) from up to two minor versions prior. Previously, restoring backups produced from even earlier versions was possible, but unreliable. Now, this operation is prevented with an error.

Lauren Hirata Singh and others added 4 commits May 15, 2023 13:01
@lnhsingh lnhsingh marked this pull request as ready for review May 15, 2023 19:24
Remove broken link until after GA release
@lnhsingh lnhsingh merged commit bad2a68 into master May 15, 2023
@lnhsingh lnhsingh deleted the 23.1-GA-RNs branch May 15, 2023 19:41
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.

7 participants