Skip to content

Commit

Permalink
Revert "sql: Add database ID to sampled query log"
Browse files Browse the repository at this point in the history
Reverts: #84195
This reverts commit 307817e.

Release note (sql change): Removes the DatabaseID field from the
`SampledQuery` telemetry log due to the potential of indefinite blocking
in the case of a lease acquisition failure.
  • Loading branch information
Thomas Hardy committed Jul 29, 2022
1 parent e10efc8 commit 53d2cd6
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 27 deletions.
1 change: 0 additions & 1 deletion docs/generated/eventlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2463,7 +2463,6 @@ contains common SQL event/execution details.
| `Database` | Name of the database that initiated the query. | no |
| `StatementID` | Statement ID of the query. | no |
| `TransactionID` | Transaction ID of the query. | no |
| `DatabaseID` | Database ID of the query. | no |
| `StatementFingerprintID` | Statement fingerprint ID of the query. | no |
| `MaxFullScanRowsEstimate` | Maximum number of rows scanned by a full scan, as estimated by the optimizer. | no |
| `TotalScanRowsEstimate` | Total number of rows read by all scans in the query, as estimated by the optimizer. | no |
Expand Down
7 changes: 1 addition & 6 deletions pkg/sql/exec_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,15 +389,14 @@ func (p *planner) maybeLogStatementInternal(
}
if telemetryMetrics.maybeUpdateLastEmittedTime(telemetryMetrics.timeNow(), requiredTimeElapsed) {
skippedQueries := telemetryMetrics.resetSkippedQueryCount()
databaseName := p.CurrentDatabase()
sampledQuery := eventpb.SampledQuery{
CommonSQLExecDetails: execDetails,
SkippedQueries: skippedQueries,
CostEstimate: p.curPlan.instrumentation.costEstimate,
Distribution: p.curPlan.instrumentation.distribution.String(),
PlanGist: p.curPlan.instrumentation.planGist.String(),
SessionID: p.extendedEvalCtx.SessionID.String(),
Database: databaseName,
Database: p.CurrentDatabase(),
StatementID: p.stmt.QueryID.String(),
TransactionID: p.txn.ID().String(),
StatementFingerprintID: uint64(stmtFingerprintID),
Expand All @@ -410,10 +409,6 @@ func (p *planner) maybeLogStatementInternal(
RowsRead: queryStats.rowsRead,
RowsWritten: queryStats.rowsWritten,
}
db, _ := p.Descriptors().GetImmutableDatabaseByName(ctx, p.txn, databaseName, tree.DatabaseLookupFlags{Required: true})
if db != nil {
sampledQuery.DatabaseID = uint32(db.GetID())
}
p.logOperationalEventsOnlyExternally(ctx, eventLogEntry{event: &sampledQuery})
} else {
telemetryMetrics.incSkippedQueryCount()
Expand Down
8 changes: 0 additions & 8 deletions pkg/sql/telemetry_logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ package sql

import (
"context"
gosql "database/sql"
"fmt"
"math"
"regexp"
Expand Down Expand Up @@ -97,14 +96,10 @@ func TestTelemetryLogging(t *testing.T) {

var sessionID string
var databaseName string
var dbID uint32

db := sqlutils.MakeSQLRunner(sqlDB)
conn := db.DB.(*gosql.DB)

db.QueryRow(t, `SHOW session_id`).Scan(&sessionID)
db.QueryRow(t, `SHOW database`).Scan(&databaseName)
dbID = sqlutils.QueryDatabaseID(t, conn, databaseName)
db.Exec(t, `SET application_name = 'telemetry-logging-test'`)
db.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.enabled = true;`)
db.Exec(t, "CREATE TABLE t();")
Expand Down Expand Up @@ -345,9 +340,6 @@ func TestTelemetryLogging(t *testing.T) {
if !strings.Contains(e.Message, "\"Database\":\""+databaseName+"\"") {
t.Errorf("expected to find Database: %s", databaseName)
}
if !strings.Contains(e.Message, "\"DatabaseID\":"+strconv.Itoa(int(dbID))) {
t.Errorf("expected to find DatabaseID: %v", dbID)
}
stmtFingerprintID := roachpb.ConstructStatementFingerprintID(tc.queryNoConstants, false, true, databaseName)
if !strings.Contains(e.Message, "\"StatementFingerprintID\":"+strconv.FormatUint(uint64(stmtFingerprintID), 10)) {
t.Errorf("expected to find StatementFingerprintID: %v", stmtFingerprintID)
Expand Down
7 changes: 7 additions & 0 deletions pkg/util/log/eventpb/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,11 @@ func readInput(
return errors.Newf("field definition must not span multiple lines: %q", line)
}

// Skip reserved fields.
if reservedDefRe.MatchString(line) {
continue
}

// A field.
if strings.HasPrefix(line, "repeated") {
line = "array_of_" + strings.TrimSpace(strings.TrimPrefix(line, "repeated"))
Expand Down Expand Up @@ -448,6 +453,8 @@ var fieldDefRe = regexp.MustCompile(`\s*(?P<typ>[a-z._A-Z0-9]+)` +
`(.*"redact:\\"safeif:(?P<safeif>([^\\]|\\[^"])+)\\"")?` +
`).*$`)

var reservedDefRe = regexp.MustCompile(`\s*(reserved ([1-9][0-9]*);)`)

func camelToSnake(typeName string) string {
var res strings.Builder
res.WriteByte(typeName[0] + 'a' - 'A')
Expand Down
9 changes: 0 additions & 9 deletions pkg/util/log/eventpb/json_encode_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions pkg/util/log/eventpb/telemetry.proto
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ message SampledQuery {
// Transaction ID of the query.
string transaction_id = 11 [(gogoproto.customname) = "TransactionID", (gogoproto.jsontag) = ',omitempty', (gogoproto.moretags) = "redact:\"nonsensitive\""];

// Database ID of the query.
uint32 database_id = 12 [(gogoproto.customname) = "DatabaseID", (gogoproto.jsontag) = ",omitempty"];

// Statement fingerprint ID of the query.
uint64 statement_fingerprint_id = 13 [(gogoproto.customname) = "StatementFingerprintID", (gogoproto.jsontag) = ',omitempty'];

Expand Down Expand Up @@ -92,6 +89,8 @@ message SampledQuery {

// The number of rows written.
int64 rows_written = 21 [(gogoproto.jsontag) = ",omitempty"];

reserved 12;
}

// CapturedIndexUsageStats
Expand Down

0 comments on commit 53d2cd6

Please sign in to comment.