sql: add json encoding/decoding support for stmt/txn stats. #67805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
This is the initial commit split off from #67090 |
This was referenced Jul 21, 2021
jordanlewis
approved these changes
Jul 22, 2021
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained
ajwerner
approved these changes
Jul 22, 2021
| encodeJSON() (json.JSON, error) | ||
| } | ||
|
|
||
| type jsonMarshaller interface { |
There was a problem hiding this comment.
nit: the stdlib spells this Marshaler.
Release note: None
|
TFTR! bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
yuzefovich
reviewed
Jul 23, 2021
There was a problem hiding this comment.
Reviewable status:
pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding.go, line 57 at r2 (raw file):
// "distsql": { "type": "boolean" }, // "failed": { "type": "boolean" }, // "opt": { "type": "boolean" },
nit: opt is always true and has been for a couple of releases now. Should we remove it everywhere?
|
@yuzefovich thanks for pointing it out! I filed this issue to track this. |
craig bot
pushed a commit
that referenced
this pull request
Aug 5, 2021
67866: sql: implement SQL Stats flush logic r=Azhng a=Azhng Previous PR: #67805 Next Chained PR: #67090 ## First Commit sql: remove `count` from stmt/txn stats system table Previously, system.statement_statistics and system.transaction_statistics table includes a `count` column that corresponds to `roachpb.StatementStatistics.Count` and `roachpb.TransactionStatistics.Count` fields respectively. The objective for that column is to make `INSERT ON CONFLICT DO UPDATE` style query easy. However, since early prototyping have shown that `INSERT ON CONFLICT DO UPDATE` style statement is quite inefficient, the SQL Stats flush mechanism will be implemented using separate queries INSERT and UPDATE statements. This column is no longer userful and it would require special handling. Removing this column simplifies the flush logic and removes the need for special handlings. Release note (sql change): count column is removed from system.statement_statistics and system.transaction_statistics tables. ## Second Commit sql: implement persistedsqlstats flush logic This commit implements the initial flush logic of the persisted sql stats subsystem. Release note: None 68426: kv: assert txn unused in SetFixedTimestamp r=nvanbenschoten a=nvanbenschoten This commit asserts that a transaction has not been used to read or to write by the time that `SetFixedTimestamp` is called on it. This was extracted from #68194 and modified to return an error from `SetFixedTimestamp` on misuse instead of fatal-ing. This provides a sufficient, temporary backstop for #68216 until the conn executor logic is fixed: ``` root@127.0.0.1:26257/movr> create table t (x int); CREATE TABLE root@127.0.0.1:26257/movr> insert into t values (1); INSERT 1 root@127.0.0.1:26257/movr> select crdb_internal_mvcc_timestamp, * from t; crdb_internal_mvcc_timestamp | x ---------------------------------+---- 1628094563935439000.0000000000 | 1 (1 row) root@127.0.0.1:26257/movr> begin as of system time (1628094563935439000.0000000000-1)::string; BEGIN root@127.0.0.1:26257/movr OPEN> select * from t; x ----- (0 rows) root@127.0.0.1:26257/movr OPEN> prepare y as select * from t as of system time 1628094563935439000.0000000000; ERROR: internal error: cannot set fixed timestamp, txn "sql txn" meta={id=e5e81c19 pri=0.01517572 epo=0 ts=1628094563.935438999,0 min=1628094563.935438999,0 seq=0} lock=false stat=PENDING rts=1628094563.935438999,0 wto=false gul=1628094563.935438999,0 already performed reads SQLSTATE: XX000 DETAIL: stack trace: github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord/txn_coord_sender.go:1016: SetFixedTimestamp() github.com/cockroachdb/cockroach/pkg/kv/txn.go:1200: SetFixedTimestamp() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:278: populatePrepared() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:220: func1() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:226: prepare() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:112: addPreparedStmt() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:570: execStmtInOpenState() github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:126: execStmt() github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1626: func1() github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1628: execCmd() github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1550: run() github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:627: ServeConn() github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:645: func1() runtime/asm_amd64.s:1371: goexit() HINT: You have encountered an unexpected error. Please check the public issue tracker to check whether this problem is already tracked. If you cannot find it there, please report the error with details by creating a new issue. If you would rather not post publicly, please contact us directly using the support form. We appreciate your feedback. root@127.0.0.1:26257/? ERROR> ``` 68442: kv: include RangeID in rangefeed goroutine stacks r=nvanbenschoten a=nvanbenschoten This commit includes the RangeID in each of a rangefeed processor and its registations' associated goroutine stacks. This is a cheap and easy way to get better observability into the ranges that have active rangefeeds. It also tells us where those goroutines are spending their time. This will also become easier to use in Go 1.17, which improved the format of stack traces. 68443: parser: add VIRTUAL syntax to help r=RaduBerinde a=rafiss Release note: None Co-authored-by: Azhng <archer.xn@gmail.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release note: None