-
Notifications
You must be signed in to change notification settings - Fork 458
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
Document viewing locking strength with EXPLAIN #7176
Conversation
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/7304bbe86e784361b1dc3d013bb11e874b498809/ Edited pages: |
@nvanbenschoten in case it helps your review, some context for the structure of only putting this on the If you are using However, if you are using I do have a question as well:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good other than the one comment about UPSERT
. Thanks Rich!
Do you think we should link to the session var for toggling SFU locking from this EXPLAIN doc section? Since folks are possibly testing/troubleshooting stuff here? My sense was "maybe" but I'm not sure if/when users should ever toggle it unless they are pretty expert, and/or working with our Support team. My sense is that linking it from here will tend to slightly "encourage" its use due to higher visibility.
My sense is that we should point this section to the implicit sfu session var / cluster setting. If people are seeing this in their explain output, they might be interested in how to toggle the behavior, if only to learn more about the functionality.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten and @rmloveland)
v20.1/explain.md, line 578 at r1 (raw file):
- [`SELECT FOR UPDATE`](select-for-update.html) - [`UPDATE`](update.html) - [`UPSERT`](upsert.html)
Unfortunately, the patch to use implicit SFU
for UPSERT didn't land in 20.1.
7304bbe
to
8bc7145
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review Nathan!
Removed the UPSERT
link.
Re: toggling SFU locking, I've added a link to the session var, but left the cluster setting out of the docs since it's non-public (but folks can still get it if they really want via SHOW ALL CLUSTER SETTINGS
as you know).
Reviewable status: complete! 0 of 0 LGTMs obtained
v20.1/explain.md, line 578 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Unfortunately, the patch to use implicit
SFU
for UPSERT didn't land in 20.1.
OK - removed.
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/8bc71450da665d54fbc8a419aa925650f2f1e62d/ Edited pages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @rmloveland)
v20.1/explain.md, line 611 at r2 (raw file):
~~~ By default, `SELECT FOR UPDATE` locking is enabled. To disable it, toggle the [`enable_implicit_select_for_update` session setting](show-vars.html#enable-implicit-select-for-update).
"is enabled for the initial row scan of UPDATE statements."
Fixes #6699, #6903, #6920. Summary of changes: - Add a new section to EXPLAIN docs called 'Find out if a statement is using SELECT FOR UPDATE locking', with an example showing how to check, and a link to the session var to disable it (mostly for visibility/learning). - List out the SQL statements that have support enabled by default for SFU locking.
8bc7145
to
344d85a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nvanbenschoten)
v20.1/explain.md, line 611 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
"is enabled for the initial row scan of UPDATE statements."
Updated!
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/344d85a468e9deacb75fb61c60b048019aca44bb/ Edited pages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @lnhsingh and @nvanbenschoten)
Fixes #6699, #6903, #6920.
Summary of changes:
Add a new section to EXPLAIN docs called 'Find out if a statement is
using SELECT FOR UPDATE locking', with an example showing how to
check.
List out the SQL statements that have support enabled by default for
SFU locking.