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: persistent SQL Stats main tracking issue #64743

Closed
22 of 24 tasks
Azhng opened this issue May 5, 2021 · 0 comments
Closed
22 of 24 tasks

sql: persistent SQL Stats main tracking issue #64743

Azhng opened this issue May 5, 2021 · 0 comments
Labels
A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
Cluster Observability

Comments

@Azhng
Copy link
Contributor

Azhng commented May 5, 2021 •
edited by cockroach-jira-scripts

This is the main tracking issue for all persisted SQL Stats related work:

Infrastructure:

Minimum Requirement to enable it by default by 21.2

Further Enhancement

Clean Up

RFC PR: #63752

Epic: CRDB-9866

Jira issue: CRDB-7226
@Azhng Azhng added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label May 5, 2021
@Azhng Azhng self-assigned this May 5, 2021
@Azhng Azhng added this to Triage in Cluster Observability via automation May 5, 2021
@Azhng Azhng mentioned this issue May 18, 2021
Merged
Azhng added a commit to Azhng/cockroach that referenced this issue May 18, 2021
@Azhng
sql: create system tables for SQL stats
285f2e5 
This PR creates two new system tables: system.sql_statement_stats
and system.sql_transaction_stats per RFC cockroachdb#63752.

This is the initial step that addresses cockroachdb#64743.

Release note: None
Azhng added a commit to Azhng/cockroach that referenced this issue May 18, 2021
@Azhng
sql: create system tables for SQL stats
7b5fb52 
This PR creates two new system tables: system.sql_statement_stats
and system.sql_transaction_stats per RFC cockroachdb#63752.

This is the initial step that addresses cockroachdb#64743.

Release note: None
Azhng added a commit to Azhng/cockroach that referenced this issue May 20, 2021
@Azhng
sql: create system tables for SQL stats
88026aa 
This PR creates two new system tables: system.sql_statement_stats
and system.sql_transaction_stats per RFC cockroachdb#63752.

This is the initial step that addresses cockroachdb#64743.

Release note: None
@kevin-v-ngo kevin-v-ngo moved this from Triage to 21.2 May in Cluster Observability May 20, 2021
Azhng added a commit to Azhng/cockroach that referenced this issue May 22, 2021
@Azhng
sql: create system tables for SQL stats
7c26f95 
This PR creates two new system tables: system.sql_statement_stats
and system.sql_transaction_stats per RFC cockroachdb#63752.

This is the initial step that addresses cockroachdb#64743.

Release note: None
@kevin-v-ngo kevin-v-ngo added the A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. label May 26, 2021
@maryliag maryliag moved this from 21.2 May to 21.2 June in Cluster Observability Jun 1, 2021
Azhng added a commit to Azhng/cockroach that referenced this issue Jun 23, 2021
@Azhng
sql: create system tables for SQL stats
4430f0c 
This PR creates two new system tables: system.statement_stats
and system.transaction_stats per RFC cockroachdb#63752.

This is the initial step that addresses cockroachdb#64743.

Release note: None
craig bot pushed a commit that referenced this issue Jul 6, 2021
@Azhng @JuanLeon1 @mgartner
Merge #65374 #67175 #67194
3fce327 
65374: sql: create system tables for SQL stats r=Azhng a=Azhng

This PR creates two new system tables: system.sql_statement_stats
and system.sql_transaction_stats per RFC #63752.

This is the initial step that addresses #64743.

Release note: None

67175: cloud: bump orchestrator version to 21.1.5 r=JuanLeon1 a=JuanLeon1



67194: sqlsmith: make TLP predicates immutable r=mgartner a=mgartner

Previously, `GenerateTLP` could generate predicates that were not
immutable. For example, a predicate could include `random()`. The TLP
method only works correctly if predicates are immutable. A non-immutable
predicate can cause TLP to erroneously report a correctness bug.

Release note: None

Co-authored-by: Azhng <archer.xn@gmail.com>
Co-authored-by: Juan Leon <github@artedo.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
@kevin-v-ngo kevin-v-ngo mentioned this issue Jul 7, 2021
Closed
@maryliag maryliag moved this from 21.2 June to 21.2 July in Cluster Observability Jul 7, 2021
@kevin-v-ngo kevin-v-ngo mentioned this issue Jul 12, 2021
Closed
@jordanlewis jordanlewis mentioned this issue Jul 13, 2021
Open
This was referenced Jul 27, 2021
Closed
Closed
@maryliag maryliag moved this from 21.2 July to 21.2 August in Cluster Observability Aug 6, 2021
@Azhng Azhng mentioned this issue Aug 6, 2021
Merged
Azhng added a commit to Azhng/cockroach that referenced this issue Aug 6, 2021
@Azhng
sql: support decode ExplainTreePlanNode from JSON format
fcbaa35 
Previously, we do not have support for decoding the
JSON-formatted ExplainTreePlanNode into the protobuf
format. This is required for us in order to implement
a crdb_internal virtual table that aggregates the cluster-wide
in-memory SQL stats and persisted SQL stats.

Related to cockroachdb#64743
Required for cockroachdb#68193

Release note: None
Azhng added a commit to Azhng/cockroach that referenced this issue Aug 10, 2021
@Azhng
sql: support decode ExplainTreePlanNode from JSON format
ecbf89a 
Previously, we do not have support for decoding the
JSON-formatted ExplainTreePlanNode into the protobuf
format. This is required for us in order to implement
a crdb_internal virtual table that aggregates the cluster-wide
in-memory SQL stats and persisted SQL stats.

Related to cockroachdb#64743
Required for cockroachdb#68193

Release note: None
@Azhng Azhng mentioned this issue Aug 10, 2021
Merged
craig bot pushed a commit that referenced this issue Aug 11, 2021
@Azhng
Merge #68555
26ebecb 
68555: sql: support decode ExplainTreePlanNode from JSON format r=Azhng a=Azhng

Previously, we do not have support for decoding the
JSON-formatted ExplainTreePlanNode into the protobuf
format. This is required for us in order to implement
a crdb_internal virtual table that aggregates the cluster-wide
in-memory SQL stats and persisted SQL stats.

Related to #64743
Required for #68193

Release note: None

Co-authored-by: Azhng <archer.xn@gmail.com>
@Azhng Azhng mentioned this issue Aug 11, 2021
Merged
craig bot pushed a commit that referenced this issue Aug 16, 2021
@yuzefovich @Azhng
@irfansharif @stevendanna
Merge #68567 #68675 #68738 #68903 #68978
f2aded5 
68567: colrpc: enhance warnings from the outbox r=yuzefovich a=yuzefovich

This commit marks several string constants as "safe" from the
redactability perspective so that the warnings logged by the outboxes
are more helpful. Additionally, several minor nits around error
formatting are addressed.

Release note: None

68675: sql: implement IterateStatementStats() for PersistedSQLStats r=maryliag a=Azhng

Depends on: #68555, #68620
Related to: #64743

Previously, IterateStatementStats() for PersistedSQLStats was
left unimplemented and it defaults to the implementation of
SQLStats.IterateStatementStats(). This means calls to
IterateStatementStats() on PersistedSQLStats cannot read
the statement statistitcs stored in system table.

This commit implements the IterateStatementStats() through
the new CombinedStmtStatsIterator which enables this method
to read both in-memory and persited statement statistics.

Release note: None

68738: sql/catalog/dbdesc: repair old descriptors corrupted due to DROPPED SCHEMA name r=ajwerner a=sajjadrizvi

#63119 fixes a bug that corrupts a database descriptor when a child schema was 
dropped, adding an entry in schema-info structure erroneously with database name
instead of schema name . Although the bug was fixed , there can be database backups 
with corrupted descriptors.

This commit adds a post-deserialization function to repair a corrupted descriptor. Moreover,
it adds a test function to ensure that descriptors with such corruption are fixed during
migration.
 
Release note: None
 
Fixes: #63148


68903: github: redirect KV code reviews to @cockroachdb/kv-prs r=irfansharif a=irfansharif

We try to use the @cockroachdb/kv alias for notifying the entire team on
issues. There's no way to disable notifications for github's automatic
codeowners driven review requests, and that tends to be a firehose, so
lets use this sub-team alias instead.

Release note: None

68978: changefeedccl: detect sink URLs with no scheme r=HonoreDB a=stevendanna

Previously, if a user provided a sink URL with no scheme (such as
` kafka%3A%2F%2Fnope%0A`), a changefeed job would be started. However,
this changefeed job would be writing into a bufferSink.  The
bufferSink is used by core changefeeds.

The user may have provided such a URL because of confusion over how to
URL encode their sink URL.

Now, they will receive an error such as

```
pq: no scheme found for sink URL "kafka%3A%2F%2Fnope%0A"
```

Release note (enterprise change): CHANGEFEED statements now error
if the provided sink URL does not contain a scheme. Such URLs are
typically a mistake and will result in non-functional changefeeds.

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Azhng <archer.xn@gmail.com>
Co-authored-by: Sajjad Rizvi <sajjad@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
craig bot pushed a commit that referenced this issue Aug 18, 2021
@rafiss @Azhng @otan
Merge #68608 #68715 #68749 #68750
472008d 
68608: go.mod: update to pgx v4.13 r=otan,knz,stevendanna a=rafiss

fixes #62840

This also required an update to cockroach-go/v2 to get the crdbpgx
transaction retry helper.

The majority of changes here are because pgx now requires the context to
be passed in always.

Other changes:
* The way that pgx prepares statements has changed -- to disable
automatic prepared statements, the PreferSimpleProtocol setting must be
enabled when making the pgxpool.
* The `noprepare` setting was removed from `workload`. This functionality
is not supported by pgx any more. pgx will either use prepared statements and
cache them automatically or use the simple protocol.
* sqlproxy tests had to be modified since pgx now has more
fallback logic to retry connections when using an sslmode of `prefer`
or `allow`. pgx also now tries to resolve `localhost` as the IPv6 address
`:::1`, which isn't well-supported by sqlproxy, so IP resolution was disabled
for sqlproxy tests.
* CHANGEFEED tests had to be changed since `pgx.Conn.Query` now immediately
tries fetching the results, which didn't work with the synchronization that was added
to TestChangefeedDataTTL.
* COPY was fixed to correctly ignore CopyData after encountering an error.

Release note: None

68715: sql: introduce crdb_internal.statement_statistics virtual table r=maryliag,ajwerner a=Azhng

Depends on  #68675
Related to: #64743

This commit introduces crdb_internal.statement_statistics virtual
table that exposes both cluster-wide in-memory statement statistics
as well as persited statement statistics. This new virtual table
will be used to replace crdb_internal.node_statement_statistics
virtual table, which only surface node-local in-memory statement
statsitics.

Release note (sql change): introduced new crdb_internal.statement_statistics
virtual table that surfaces both cluster-wide in-memory statement statistics
as well as persited statement statistics.

68749: builtins: implement a separate session_user() builtin r=richardjcai a=otan

This is needed groundwork to separate current_user and session_user.

Refs: #15005

Release note (sql change): Add a session_user() builtin function, which
currently returns the same thing as current_user() as we do not
implement `SET ROLE`.

68750: parser: SET ROLE syntax preparation r=richardjcai a=otan

See commits for details.

Refs: #15005

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Azhng <archer.xn@gmail.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@Azhng Azhng changed the title sql: persist SQL statement stats into system table sql: persistent SQL Stats main tracking issue Aug 24, 2021
This was referenced Sep 1, 2021
Closed
Closed
@maryliag maryliag moved this from 21.2 October to 21.2 September in Cluster Observability Oct 4, 2021
@maryliag maryliag moved this from 21.2 September to 21.2 October in Cluster Observability Oct 7, 2021
@maryliag maryliag moved this from 22.2 to Backlog in Cluster Observability Sep 23, 2022
@maryliag maryliag closed this as completed Nov 3, 2023
Cluster Observability automation moved this from Backlog to Done Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
No open projects
Cluster Observability
Done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@maryliag @Azhng @kevin-v-ngo