server: tenant span stats for TableStats, TableDetails & DatabaseDetails endpoints #90267
Assignees
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Comments
THardy98
added
C-enhancement
This was referenced Oct 19, 2022
|
@knz @dhartunian does the issue description make sense, any thoughts on approach? Any other stakeholders that would be beneficial to loop in here? |
|
it's pretty close, yes. Thanks. Note that @aayushshah15 is currently researching solutions for an adjacent area and might have additional thoughts to share. |
|
cc @nkodali for visibility/thoughts as well |
|
cross-ref #84105 |
dhartunian
added a commit
to dhartunian/cockroach
that referenced
this issue
Feb 28, 2023
Replaced usages of `TODOSQLCodec` with the codec from `sqlServer.execCfg`. This enables the DB and Table stats endpoints to work from tenants. Resolves: cockroachdb#82879 Relates to: cockroachdb#90261, cockroachdb#90267, cockroachdb#90268, cockroachdb#90264, cockroachdb#89429 Epic: CRDB-12100 Release note: None
craig bot
pushed a commit
that referenced
this issue
Mar 1, 2023
95141: storage: Add support for TargetBytes for EndTxn r=nvanbenschoten a=KaiSun314 Fixes: #77228 Intent resolution batches are sequenced on raft and each batch can consist of 100-200 intents. If an intent key or even value in some cases are large, it is possible that resolving all intents in the batch would result in a raft command size exceeding the max raft command size kv.raft.command.max_size. To address this, we add support for TargetBytes in resolve intent and resolve intent range commands, allowing us to stop resolving intents in the batch as soon as we exceed the TargetBytes max bytes limit. This PR adds byte pagination for synchronous intent resolution (i.e. EndTxn / End Transaction). Release note: None 97511: status: set codec from context in table stats requests r=knz a=dhartunian Replaced usages of `TODOSQLCodec` with the codec from `sqlServer.execCfg`. This enables the DB and Table stats endpoints to work from tenants. Resolves: #82879 Relates to: #90261, #90267, #90268, #90264, #89429 Epic: CRDB-12100 Release note: None 97657: sql,mon: expose the memory monitors as virtual table r=yuzefovich a=yuzefovich This commit adjusts our memory monitors to be able to traverse the whole monitor tree starting at the root monitor. In particular, this commit introduces a doubly-linked list of "siblings" and stores the reference to the head in the parent. Whenever a monitor is `Start`ed, it is included as the new head of its parent's children list, whenever a monitor is `Stop`ped, it is removed from that list. The overhead of this additional tracking should be negligible since only the parent's lock needs to be acquired twice throughout the lifetime of a monitor (thus, assuming relatevily long-lived sessions, this wouldn't affect the root monitor) and the increase in allocations is minor. This required clarification on how locks on a parent and a child can be held at the same time. In particular, since the main code path is acquiring locks "upwards" (meaning when growing the child's budget we might need to grow the parent's budget, and "growing" locks the corresponding monitor), whenever we want to traverse the tree from the root down, we have to unlock the parent's monitor before recursing into the children. As a result, the traversal might give us an inconsistent view (where a recently stopped child can contribute to the usage of the parent while we don't recurse into that child). This seems acceptable. This ability to traverse the whole monitor tree is now exposed as a new virtual table `crdb_internal.node_memory_monitors` which includes a line for each monitor active at the time of table generation (subject to possible inconsistency mentioned above). The table includes the name of the monitors which can be suggestive about the activity on the cluster, thus, access to this table is gated on the "view activity" permissions. The usage of the virtual table to expose the memory monitors information results in flattening of the tree; however, one of the fields is a "level" (or "generation") in relation to the root, plus the ordering of rows is very specific, so we can still format the output to see the hierarchy. We also assign IDs to the monitors (which is their pointer address). Exposing this information as a virtual table allows us to use SQL to analyze it. Here is one example of visualizing it: ``` [email protected]:26257/defaultdb> SELECT repeat(' ', level) || name || ' ' || crdb_internal.humanize_bytes(used) FROM crdb_internal.node_memory_monitors; ?column? ------------------------------------------------------------------- root 0 B internal-planner.‹root›.‹resume-job-101› 0 B internal-planner.‹node›.‹resume-job-100› 0 B internal-planner.‹node›.‹resume-job-842810460057567233› 0 B sql 900 KiB session root 20 KiB txn 10 KiB flow e595eb80 10 KiB session 0 B txn-fingerprint-id-cache 0 B internal SQL executor 0 B internal SQL executor 0 B internal sql executor 0 B conn 105 KiB internal SQL executor 70 KiB internal SQL executor 60 KiB SQLStats 540 KiB SQLStats 0 B distsql 0 B server-cache-mon 0 B bulk-mon 0 B backup-mon 0 B backfill-mon 0 B pre-conn 105 KiB closed-session-cache 190 KiB timeseries-results 0 B timeseries-workers 0 B kv-mem 20 KiB rangefeed-monitor 0 B rangefeed-system-monitor 0 B (30 rows) ``` There are a couple of additional minor improvements: - we now include the short FlowID into the flow's memory monitor name. Combined with the distsql_flows virtual table we'll be able to get the stmt fingerprint for the remote flows running on a node. - new `crdb_internal.humanize_bytes` builtin function is introduced. Note that the corresponding `cluster_memory_monitors` virtual table is not introduced out of caution. In particular, this would lead to RPCs issued to all nodes in the cluster, and since each node can have on the order of hundreds of thousands monitors, the response to each RPC could have non-trivial network cost. We can revisit this decision later if we find that a cluster level view of the memory monitors is desirable, but for now a node level view seems like a big improvement on its own. Addresses: #35097. Fixes: #90551. Release note (sql change): New internal virtual table `crdb_internal.memory_monitors` is introduced. It exposes all of the current reservations with the memory accounting system on a single node. Access to the table requires VIEWACTIVITY or VIEWACTIVITYREDACTED permissions. 97853: builtins: fix crdb_internal.hide_sql_constants array overload r=xinhaoz a=xinhaoz Previously, erroring on parsing a stmt provided in one of the array elements to crdb_internal.hide_sql_constants would result in an error. This commit ensures that the empty string is returned for an unparseable stmt. Epic: none Release note: None Co-authored-by: Kai Sun <[email protected]> Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Xin Hao Zhang <[email protected]>
|
Closed with the merge of: #96223 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Part of: #89429
Currently, the
TableStatsandDatabaseDetailsendpoints surface span statistics usingstatsForSpanwhich makes use of thekv.DB, and theTableDetailsendpoint surfaces range count usingCountRangesfrom thekvcoord.DistSender.Use of these KV operations are blockers from making these endpoints tenant-scoped, as tenants will not have access to the KV layer.
To me, there seem to be a couple options to do this:
Alternative approaches and discussion welcome.
Jira issue: CRDB-20665
Epic: CRDB-16704
The text was updated successfully, but these errors were encountered: