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.

Sign up for GitHub

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

Jump to bottom

sql: consider prohibiting CREATE STATISTICS without specifying AOST option #72719

Closed
yuzefovich opened this issue Nov 13, 2021 · 2 comments · Fixed by #124488
Closed

sql: consider prohibiting CREATE STATISTICS without specifying AOST option #72719

yuzefovich opened this issue Nov 13, 2021 · 2 comments · Fixed by #124488
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented Nov 13, 2021 •
edited by cockroach-jira-scripts
Loading

For CREATE STATISTICS statement we get a different internal behavior depending on whether AS OF SYSTEM TIME option is specified or not:

  • if it isn't, then we perform a regular scan that might contend with concurrent txns (which might have a performance hit, but also might "starve" the stats job)
  • if it is, then we do an "inconsistent" scan.

We've seen multiple cases when users didn't know about this difference and shot themselves in the foot. I think that it might be a good idea to error out if AOST is not specified and give a hint to use AOST -0.001s (as we already do for ANALYZE).

Jira issue: CRDB-11266
@yuzefovich yuzefovich added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Nov 13, 2021
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Nov 13, 2021
@yuzefovich yuzefovich added the E-quick-win Likely to be a quick win for someone experienced. label Nov 23, 2021
@yuzefovich
Copy link
Member Author

We probably should keep some way of running CREATE STATISTICS at the current time, at least for testing. Maybe use AS OF SYSTEM TIME '-0s' for that.

@yuzefovich yuzefovich mentioned this issue Oct 28, 2022
Closed
@mgartner mgartner moved this to New Backlog in SQL Queries Jul 24, 2023
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Uzair5162 added a commit to Uzair5162/cockroach that referenced this issue May 21, 2024
@Uzair5162
sql: prohibit CREATE STATISTICS without specifying AOST option
990b896 
Makes specifying an AS OF SYSTEM TIME option required when using the
CREATE STATISTICS command. Using CREATE STATISTICS without an AOST
option results in a regular scan which could contend with concurrent
transactions.

Fixes: cockroachdb#72719

Release note (sql change): using the CREATE STATISTICS command without
the AS OF SYSTEM TIME option could contend with concurrent transactions
and cost performance. The AS OF SYSTEM TIME option is now required to
prevent such usage.
@Uzair5162 Uzair5162 mentioned this issue May 21, 2024
Merged
Uzair5162 added a commit to Uzair5162/cockroach that referenced this issue May 22, 2024
@Uzair5162
sql: add a default AOST option to CREATE STATISTICS
4bd76b6 
Using CREATE STATISTICS without an AOST option results in a regular scan
which could contend with concurrent transactions. Implements a default
AOST of -1us to the CREATE STATISTICS command to avoid this.

Fixes: cockroachdb#72719

Release note (sql change): using the CREATE STATISTICS command without
the AS OF SYSTEM TIME option could contend with concurrent transactions
and cost performance. Running CREATE STATISTICS without specifying AS OF
SYSTEM TIME now uses a default of -1us.
Uzair5162 added a commit to Uzair5162/cockroach that referenced this issue May 22, 2024
@Uzair5162
sql: add a default AOST option to CREATE STATISTICS
696d48d 
Using CREATE STATISTICS without an AOST option results in a regular scan
which could contend with concurrent transactions. Implements a default
AOST of -1us to the CREATE STATISTICS command to avoid this.

Fixes: cockroachdb#72719

Release note (sql change): using the CREATE STATISTICS command without
the AS OF SYSTEM TIME option could contend with concurrent transactions
and cost performance. Running CREATE STATISTICS without specifying AS OF
SYSTEM TIME now uses a default of -1us.
Uzair5162 added a commit to Uzair5162/cockroach that referenced this issue May 22, 2024
@Uzair5162
sql: add a default AOST option to CREATE STATISTICS
937da4c 
Using CREATE STATISTICS without an AOST option results in a regular scan
which could contend with concurrent transactions. This commit adds a
default AOST option of -1us to the CREATE STATISTICS command to avoid
this.

Fixes: cockroachdb#72719

Release note (sql change): using the CREATE STATISTICS command without
the AS OF SYSTEM TIME option could contend with concurrent transactions
and cost performance. Running CREATE STATISTICS without specifying AS OF
SYSTEM TIME now uses a default of -1us.
craig bot pushed a commit that referenced this issue May 23, 2024
@Uzair5162
Merge #124488
565fa54 
124488: sql: add a default AOST option to CREATE STATISTICS r=Uzair5162 a=Uzair5162

Using CREATE STATISTICS without an AOST option results in a regular scan which could contend with concurrent transactions. This commit adds a default AOST option of -1us to the CREATE STATISTICS command to avoid this.

Fixes: #72719

Release note (sql change): using the CREATE STATISTICS command without the AS OF SYSTEM TIME option could contend with concurrent transactions and cost performance. Running CREATE STATISTICS without specifying AS OF SYSTEM TIME now uses a default of -1us.

Co-authored-by: Uzair Ahmad <[email protected]>
@craig craig bot closed this as completed in 805b682 May 23, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in SQL Queries May 23, 2024
@yuzefovich yuzefovich mentioned this issue Jul 20, 2024
Merged
craig bot pushed a commit that referenced this issue Jul 26, 2024
@yuzefovich
Merge #127583 #127672
59bb8eb 
127583: sql: include MaxTimestampAge info for TableReader in DistSQL diagrams r=yuzefovich a=yuzefovich

This commit includes `MaxTimestampAge` field of the `TableReaderSpec` into the DistSQL diagram. This feature is used by the table stats collection to utilize the inconsistent scan, and it'll be easier to confirm that it's actually used under the hood.

Informs: #72719.
Epic: None

Release note: None

127672: logictest: retry relocate stmt in a couple of places r=yuzefovich a=yuzefovich

The capabilities are propagated asynchronously, so previously we could try to relocate ranges in the secondary tenant before the necessary capability was picked up. This was recently changed in 5f2a4f8.

Fixes: #127659.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: add a default AOST option to CREATE STATISTICS Uzair5162/cockroach
1 participant
@yuzefovich