server: make the DatabaseDetails endpoint tenant-scoped #90261
Assignees
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Comments
THardy98
added
C-enhancement
THardy98
changed the title
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]>
craig bot
pushed a commit
that referenced
this issue
Mar 15, 2023
93209: cluster-ui: move database details endpoint to use sql-over-http r=THardy98 a=THardy98 Part of: #89429 Addresses: #90261 This change moves the existing database details endpoint to use sql-over-http. The data for database details is populated using 7 queries, which fetch the database's: - ID - grants - tables - replicas and regions - index usage statistics - zone configuration - span statistics This PR also changes the structure of the database details response. Each field within the overlying database details response encapsulates the response of one of the queries. Query failures are captured at the top-level of the response and within the corresponding response field for the query (scoping query failures to each transaction). Additionally, we no longer fetch `missing_tables` as we are no longer fetching table stats per table, but in a single query. **DEMO** https://www.loom.com/share/3a51bf5a8f3c41089fc5acae9ff29fcf Release note: None 98651: execinfra: explicitly pass the output RowReceiver to Processor.Run r=yuzefovich a=yuzefovich **colflow: simplify the vectorized flow creator a bit** This commit refactors the vectorized flow creator to directly embed two structs that implement interfaces used by the creator. This allows for those structs to not be allocated on the main code path. Also, `flowCreatorHelper` no longer implements the `Releasable` interface. Release note: None **execinfra: explicitly pass the output RowReceiver to Processor.Run** This commit modifies how the output of a processor is plumbed through. Previously, we would pass it on the processor construction and store in the `ProcessorBaseNoHelper` to later be used when calling `execinfra.Run` method. However, the upcoming work on multiple active portals will require updating the output when resuming the flow, so this commit makes it so that we now pass the output explicitly into `execinfra.Processor.Run` method. This will make the work on multiple active portals cleaner, but also this change seems like a good one independent of that because we're able to store all outputs in the single place (`FlowBase.outputs`). Epic: CRDB-17622 Release note: None 98678: cloudccl/gcp: use correct testing.T in subtests r=dt a=dt Release note: none. Epic: none. Co-authored-by: Thomas Hardy <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: David Taylor <[email protected]>
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
Blockers: #82879, #90267
As part of our effort to migrate to a multi-tenant architecture, we need to ensure our existing database details endpoint is tenant-scoped.
Unfortunately, the
DatabaseDetailsendpoint makes use of two functions that block our ability to scope to tenants:getDatabaseTableSpans->generateTableScan:generateTableScandoes not decode with the proper tenant prefix, instead usingTODOSQLCodec. There is an issue filed for this already: server: the TableDetails, TableStats, DatabaseDetails API don't decode with a proper tenant prefix #82879, which will need to be resolved to unblock this issue.getDatabaseStats->statsForSpan:statsForSpanmakes use of the KV layer which tenants will not have access to, there is an issue filed for this already: server: tenant span stats for TableStats, TableDetails & DatabaseDetails endpoints #90267 which will need to be resolved to unblock this issue.To maintain all information we provide currently, the blockers above would need to be resolved to allow the endpoint to be tenant-scoped.
Alternatively, If we choose to initially omit database statistics from the
DatabaseDetailsendpoint:cockroach/pkg/server/admin.go
Line 519 in 669e9a5
we can immediately work on tenant-scoping the endpoint, with the idea to resolve the blockers above in the future.
Jira issue: CRDB-20662
Epic CRDB-16704
The text was updated successfully, but these errors were encountered: