Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
69238: server: create api to query persisted stats by date range r=xinhaoz a=xinhaoz

This commit creates a new API endpoint /_status/combinedstmts
to fetch combined in-memory and persisted statements and
transactions from crdb_internal.statement_statistics
and crdb_internal.transaction_statistics. The request
supports optional start and end parameters which
represent the unix time at which the data was aggregated.

The parameteres start, end and combined have also been
added to the StatementsRequest message. Setting combined
to true will forward the request to fetch data from the
new combined api, using the start and end parameters
provided.

Release justification: Category 2 low-risk updates to new
functionality

Release note (api change): New endpoint /_status/combinedstmts
to retrieve persisted and in-memory statements from
crdb_internal.statement_statistics and
crdb_internal.transaction_statistics by aggregated_ts
range. The request supports optional query string
parameters start and end, which are the date range in unix
time. The response returned is currently the response
expected from /_status/statements.

/_status/statements has also been udpated to support
the parameters combined, start, and end.
If combined is true, then the statements endpoint will
use /_status/combinedstmts with the optional parameters
start and end.

69395: opt: support locality optimized search for scans with more than 1 row r=yuzefovich a=rytaft

This commit updates the logic for planning locality optimized search to
allow the optimization the be planned if there are fewer than 100,000
keys selected.

The optimization is not yet supported for scans with a hard limit.

Informs #64862

Release justification: Low risk, high benefit change to existing functionality.

Release note (performance improvement): locality optimized search
is now supported for scans that are guaranteed to return 100000 keys
or less. This optimization allows the execution engine to avoid
visiting remote regions if all requested keys are found in the local
region, thus reducing the latency of the query.

69469: ts: include histogram quantiles in tsdump r=dhartunian a=tbg

`cockroach debug tsdump` previously silently did not return metrics
backed by histograms. This is for technical reasons related to the
bookkeeping of metrics names and is rectified here by requiring some
extra tagging of metrics that are histograms so that they can be picked
up by tsdump. It's not pretty, but pragmatic: it works and it'll be
clear to anyone adding a histogram in the future how to proceed, even if
they may wonder why things work in such a roundabout manner (and if
they're curious about that, the relevant issues are linked in comments
as well).

I also renamed AllMetricsNames to AllInternalTimeseriesMetricsNames
to make clear what is being returned.

Demo:

```
killall -9 cockroach; rm -rf cockroach-data;
./cockroach start-single-node --insecure --background && \
./cockroach workload run kv --init \
    'postgres://[email protected]:26257?sslmode=disable' --duration=300s && \
./cockroach debug tsdump --format=raw --insecure > tsdump.gob && \
killall -9 cockroach && rm -rf cockroach-data && \
COCKROACH_DEBUG_TS_IMPORT_FILE=tsdump.gob ./cockroach start-single-node --insecure
```

![image](https://user-images.githubusercontent.com/5076964/131134624-b5471621-d23b-4ce7-9026-e8aeb3613231.png)

Release justification: low-risk observability fix
Release note (ops change): The ./cockroach debug tsdump command now
downloads histogram timeseries it silently omitted previously.

Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
  • Loading branch information
4 people committed Aug 28, 2021
4 parents 392bd80 + 63eb73a + 3af8181 + 4cbe4db commit b8223fb
Show file tree
Hide file tree
Showing 52 changed files with 2,031 additions and 658 deletions.
96 changes: 96 additions & 0 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,9 @@ tenant pods.
| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| node_id | [string](#cockroach.server.serverpb.StatementsRequest-string) | | | [reserved](#support-status) |
| combined | [bool](#cockroach.server.serverpb.StatementsRequest-bool) | | If this field is set we will use the combined statements API instead. | [reserved](#support-status) |
| start | [int64](#cockroach.server.serverpb.StatementsRequest-int64) | | These fields are used for the combined statements API. | [reserved](#support-status) |
| end | [int64](#cockroach.server.serverpb.StatementsRequest-int64) | | | [reserved](#support-status) |



Expand Down Expand Up @@ -2861,6 +2864,99 @@ tenant pods.
| ----- | ---- | ----- | ----------- | -------------- |
| key_data | [cockroach.sql.StatementStatisticsKey](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.StatementStatisticsKey) | | | [reserved](#support-status) |
| node_id | [int32](#cockroach.server.serverpb.StatementsResponse-int32) | | | [reserved](#support-status) |
| aggregated_ts | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementsResponse-google.protobuf.Timestamp) | | | [reserved](#support-status) |





<a name="cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedCollectedTransactionStatistics"></a>
#### StatementsResponse.ExtendedCollectedTransactionStatistics



| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| stats_data | [cockroach.sql.CollectedTransactionStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.CollectedTransactionStatistics) | | | [reserved](#support-status) |
| node_id | [int32](#cockroach.server.serverpb.StatementsResponse-int32) | | | [reserved](#support-status) |






## CombinedStatementStats

`GET /_status/combinedstmts`

Retrieve the combined in-memory and persisted statement stats by date range.

Support status: [reserved](#support-status)

#### Request Parameters







| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| start | [int64](#cockroach.server.serverpb.CombinedStatementsStatsRequest-int64) | | Unix time range for aggregated statements. | [reserved](#support-status) |
| end | [int64](#cockroach.server.serverpb.CombinedStatementsStatsRequest-int64) | | | [reserved](#support-status) |







#### Response Parameters







| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| statements | [StatementsResponse.CollectedStatementStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.CollectedStatementStatistics) | repeated | | [reserved](#support-status) |
| last_reset | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementsResponse-google.protobuf.Timestamp) | | Timestamp of the last stats reset. | [reserved](#support-status) |
| internal_app_name_prefix | [string](#cockroach.server.serverpb.StatementsResponse-string) | | If set and non-empty, indicates the prefix to application_name used for statements/queries issued internally by CockroachDB. | [reserved](#support-status) |
| transactions | [StatementsResponse.ExtendedCollectedTransactionStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedCollectedTransactionStatistics) | repeated | Transactions is transaction-level statistics for the collection of statements in this response. | [reserved](#support-status) |






<a name="cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.CollectedStatementStatistics"></a>
#### StatementsResponse.CollectedStatementStatistics



| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| key | [StatementsResponse.ExtendedStatementStatisticsKey](#cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedStatementStatisticsKey) | | | [reserved](#support-status) |
| id | [uint64](#cockroach.server.serverpb.StatementsResponse-uint64) | | | [reserved](#support-status) |
| stats | [cockroach.sql.StatementStatistics](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.StatementStatistics) | | | [reserved](#support-status) |





<a name="cockroach.server.serverpb.StatementsResponse-cockroach.server.serverpb.StatementsResponse.ExtendedStatementStatisticsKey"></a>
#### StatementsResponse.ExtendedStatementStatisticsKey



| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| key_data | [cockroach.sql.StatementStatisticsKey](#cockroach.server.serverpb.StatementsResponse-cockroach.sql.StatementStatisticsKey) | | | [reserved](#support-status) |
| node_id | [int32](#cockroach.server.serverpb.StatementsResponse-int32) | | | [reserved](#support-status) |
| aggregated_ts | [google.protobuf.Timestamp](#cockroach.server.serverpb.StatementsResponse-google.protobuf.Timestamp) | | | [reserved](#support-status) |



Expand Down
34 changes: 34 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/regional_by_row
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,40 @@ SELECT * FROM [EXPLAIN SELECT * FROM regional_by_row_table WHERE pk = 1] OFFSET
table: regional_by_row_table@primary
spans: [/'ap-southeast-2'/1 - /'ap-southeast-2'/1] [/'us-east-1'/1 - /'us-east-1'/1]

# Query with more than one key.
query T
SELECT * FROM [EXPLAIN SELECT * FROM regional_by_row_table WHERE pk IN (1, 4)] OFFSET 2
----
·
• union all
│ limit: 2
├── • scan
│ missing stats
│ table: regional_by_row_table@primary
│ spans: [/'ap-southeast-2'/1 - /'ap-southeast-2'/1] [/'ap-southeast-2'/4 - /'ap-southeast-2'/4]
└── • scan
missing stats
table: regional_by_row_table@primary
spans: [/'ca-central-1'/1 - /'ca-central-1'/1] [/'ca-central-1'/4 - /'ca-central-1'/4] [/'us-east-1'/1 - /'us-east-1'/1] [/'us-east-1'/4 - /'us-east-1'/4]

statement ok
SET tracing = on,kv,results; SELECT * FROM regional_by_row_table WHERE pk IN (1, 4); SET tracing = off

# Both rows are found in the local region, so the other regions are not searched.
query T
SELECT message FROM [SHOW KV TRACE FOR SESSION] WITH ORDINALITY
WHERE message LIKE 'fetched:%' OR message LIKE 'output row%'
OR message LIKE 'Scan%'
ORDER BY ordinality ASC
----
Scan /Table/73/1/"@"/1/0, /Table/73/1/"@"/4/0
fetched: /regional_by_row_table/primary/'ap-southeast-2'/1/pk2/a/b/j -> /1/2/3/'{"a": "b"}'
output row: [1 1 2 3 '{"a": "b"}']
fetched: /regional_by_row_table/primary/'ap-southeast-2'/4/pk2/a/b/j -> /4/5/6/'{"c": "d"}'
output row: [4 4 5 6 '{"c": "d"}']

# Tests using locality optimized search for lookup joins (including foreign
# key checks).
statement ok
Expand Down
23 changes: 23 additions & 0 deletions pkg/ccl/serverccl/tenant_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,20 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
tenantStats, err := tenantStatusServer.Statements(ctx, request)
require.NoError(t, err)

combinedStatsRequest := &serverpb.CombinedStatementsStatsRequest{}
tenantCombinedStats, err := tenantStatusServer.CombinedStatementStats(ctx, combinedStatsRequest)
require.NoError(t, err)

path := "/_status/statements"
var nonTenantStats serverpb.StatementsResponse
err = serverutils.GetJSONProto(nonTenant, path, &nonTenantStats)
require.NoError(t, err)

path = "/_status/combinedstmts"
var nonTenantCombinedStats serverpb.StatementsResponse
err = serverutils.GetJSONProto(nonTenant, path, &nonTenantCombinedStats)
require.NoError(t, err)

checkStatements := func(tc []testCase, actual *serverpb.StatementsResponse) {
var expectedStatements []string
for _, stmt := range tc {
Expand Down Expand Up @@ -149,9 +158,11 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {

// First we verify that we have expected stats from tenants
checkStatements(testCaseTenant, tenantStats)
checkStatements(testCaseTenant, tenantCombinedStats)

// Now we verify the non tenant stats are what we expected.
checkStatements(testCaseNonTenant, &nonTenantStats)
checkStatements(testCaseNonTenant, &nonTenantCombinedStats)

// Now we verify that tenant and non-tenant have no visibility into each other's stats.
for _, tenantStmt := range tenantStats.Statements {
Expand All @@ -165,4 +176,16 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
require.NotEqual(t, tenantTxn, nonTenantTxn, "expected tenant to have no visibility to non-tenant's transaction stats, but found:", nonTenantTxn)
}
}

for _, tenantStmt := range tenantCombinedStats.Statements {
for _, nonTenantStmt := range nonTenantCombinedStats.Statements {
require.NotEqual(t, tenantStmt, nonTenantStmt, "expected tenant to have no visibility to non-tenant's statement stats, but found:", nonTenantStmt)
}
}

for _, tenantTxn := range tenantCombinedStats.Transactions {
for _, nonTenantTxn := range nonTenantCombinedStats.Transactions {
require.NotEqual(t, tenantTxn, nonTenantTxn, "expected tenant to have no visibility to non-tenant's transaction stats, but found:", nonTenantTxn)
}
}
}
3 changes: 3 additions & 0 deletions pkg/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_library(
"authentication.go",
"auto_tls_init.go",
"auto_upgrade.go",
"combined_statement_stats.go",
"config.go",
"config_unix.go",
"config_windows.go",
Expand Down Expand Up @@ -150,6 +151,7 @@ go_library(
"//pkg/sql/sqlliveness",
"//pkg/sql/sqlliveness/slprovider",
"//pkg/sql/sqlstats",
"//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil",
"//pkg/sql/sqlutil",
"//pkg/sql/stats",
"//pkg/sql/stmtdiagnostics",
Expand Down Expand Up @@ -381,6 +383,7 @@ go_test(
"@com_github_grpc_ecosystem_grpc_gateway//runtime:go_default_library",
"@com_github_kr_pretty//:pretty",
"@com_github_lib_pq//:pq",
"@com_github_prometheus_client_model//go",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
"@org_golang_google_grpc//:go_default_library",
Expand Down
Loading

0 comments on commit b8223fb

Please sign in to comment.