Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: make statement fingerprint string more generalized #120409

Closed
xinhaoz opened this issue Mar 13, 2024 · 0 comments · Fixed by #120507
Closed

sql: make statement fingerprint string more generalized #120409

xinhaoz opened this issue Mar 13, 2024 · 0 comments · Fixed by #120507
Assignees
Labels
A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-observability

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Mar 13, 2024

These are some small improvements that can be made to the statement fingerprinting process that will reduce cardinality:

  • format literals and placeholders into the same representation (e.g. _, '_', $1 can all be formatted into the same character, _ to prevent generating different fingerprints when a combination of constants, string literals and placeholders are used in the same places.
  • Shorten long lists to one element and remove length range. Currently long lists are shortened to their first two elements and a special string containing information about the length of the rest of the list. Lists are also only shortened if they only contain literals/placeholders. (e.g. (1, 2, 3, $1) -> (_, , more1_10)but the following is not shortened(1, 2+2, 3, 4) -> (, +, _, _)`). We can more aggressively shorten lists that contain literals/placeholders or simple subexpressions of only literals and placeholders to their first element, and also remove the range indicators to further generalize such queries. This would bucket the following queries together:
SELECT a from foo WHERE b IN (1, 2, 3, 4, 5, 6, ... 50)

SELECT a from foo WHERE b IN (1, 2, 3, 4, 5, 6, ... 100)

SELECT a from foo WHERE b IN (1, 2, 3, '4'::INT, 5, 6, ... 50)

etc.

Relates to #99220.

Jira issue: CRDB-36676

Epic CRDB-37544

@xinhaoz xinhaoz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Mar 13, 2024
@xinhaoz xinhaoz self-assigned this Mar 13, 2024
cockroach-dev-inf pushed a commit that referenced this issue Mar 15, 2024
This commit introduces a way to add additional format flags when
formatting a statement AST into its statement fingerprint representation
for sql stats. This allows us to more aggressively generalize the
statement fingerprint.

`sql.stats.statement_fingerprint.format_mask` will be used to supply
these additional flags to the formatter.  It is currently 0 by default
since no new flags for fingerprints have been added.

Epic: none
Part of: #120409

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
Add functions for the `FmtForFingerprint` flag to shorten long lists
to a representative two element list where the second element is simply
`__more__` to represent the rest of the list.

Specifically, where `FmtHideConstants` would shorten tuples, arrays and
VALUES clauses to its first two elements if it coontains only literals
and placeholders, `FmtCollapseLists` shortens these lists to its first
element if all items in the list are any combination of placeholders,
literals, or a simple subexpressions containing only literals and/or
placeholders.

Enabling this behaviour for statement fingerprint generation can be done
via setting the cluster setting `sql.stats.statement_fingerprint.format_mask`
to include `FmtCollapseLists`.

```
FmtHideConstants (previous behaviour):
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, _), (_, _)), ((_, _), (_, _)), ((_, _), (_, _))

ARRAY[1, 2, 3+4] ->
ARRAY[_, _, _+4]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, 2, __more1_10__)

---

FmtCollapseLists:
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, __more__), __more__), (__more__)

ARRAY[1, 2, 3+4] ->
ARRAY[_, __more__]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, __more__)
```

Epic: none
Part of: cockroachdb#120409

Release note (sql change): If `sql.stats.statement_fingerprint.format_mask`
contains the `FmtCollapseLists` bit mask, then lists in sql statements that
contain only constant expressions (placeholders and/or literals or simple
exprs containing only placeholders/literals) will be shortened to their
first element (scrubbed), with no information about the length of the
rest of the list. This fingerprint is used in sql execution statistics tracking
and will allow a higher rate of similar queries to be tracked together.  E.g.
```
The following 3 statements will generate the same fingerprint:
SELECT * FROM foo WHERE pair IN ((1, 2), (3, 4))
SELECT * FROM foo WHERE pair IN ((1, 2), (3, 4), (5, 6))
SELECT * FROM foo WHERE pair IN ((1, 2), (3*3, 4+4), ($1, $2))
->
SELECT * FROM foo WHERE pair IN ((_, _), __more__)

The following 2 statements will generate the same fingerprint:
INSERT INTO foo VALUES (1, 2), (3, 4), (5, 6)
INSERT INTO foo VALUES (1, 2), (3, 4), (5, 6) ... (100, 101)
->
INSERT INTO foo VALUES (_, _), (__more__)

```
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
The new AST format flag  `FmtConstantsAsUnderscores` can be used
to format placeholders and literals as the same char to reduce any potential
cardinality from variances in formatting string literals, numbers, and
placeholders. FmtConstantsAsUnderscores will replace all placeholders, literals,
and string literals with `_`.

For example:
```
FmtHideConstants:
UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = $1

UPDATE foo SET a = '1', b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = '_', b = _, c = _ WHERE d = $1

UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = 5 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = _

FmtConstantsAsUnderscores will produce the following for all 3:
UPDATE foo SET a = _, b = _, c = _ WHERE d = _
```

Epic: none
Part of: cockroachdb#120409

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
Add functions for the `FmtForFingerprint` flag to shorten long lists
to a representative two element list where the second element is simply
`__more__` to represent the rest of the list.

Specifically, where `FmtHideConstants` would shorten tuples, arrays and
VALUES clauses to its first two elements if it coontains only literals
and placeholders, `FmtCollapseLists` shortens these lists to its first
element if all items in the list are any combination of placeholders,
literals, or a simple subexpressions containing only literals and/or
placeholders.

Enabling this behaviour for statement fingerprint generation can be done
via setting the cluster setting `sql.stats.statement_fingerprint.format_mask`
to include `FmtCollapseLists`.

```
FmtHideConstants (previous behaviour):
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, _), (_, _)), ((_, _), (_, _)), ((_, _), (_, _))

ARRAY[1, 2, 3+4] ->
ARRAY[_, _, _+4]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, 2, __more1_10__)

---

FmtCollapseLists:
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, __more__), __more__), (__more__)

ARRAY[1, 2, 3+4] ->
ARRAY[_, __more__]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, __more__)
```

Epic: none
Part of: cockroachdb#120409

Release note: none
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
This commit introduces a way to add additional format flags when
formatting a statement AST into its statement fingerprint representation
for sql stats. This allows us to more aggressively generalize the
statement fingerprint.

`sql.stats.statement_fingerprint.format_mask` will be used to supply
these additional flags to the formatter.  It is currently 0 by default
since no new flags for fingerprints have been added.

Epic: none
Part of: cockroachdb#120409

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
Add functions for the `FmtForFingerprint` flag to shorten long lists
to a representative two element list where the second element is simply
`__more__` to represent the rest of the list.

Specifically, where `FmtHideConstants` would shorten tuples, arrays and
VALUES clauses to its first two elements if it coontains only literals
and placeholders, `FmtCollapseLists` shortens these lists to its first
element if all items in the list are any combination of placeholders,
literals, or a simple subexpressions containing only literals and/or
placeholders.

Enabling this behaviour for statement fingerprint generation can be done
via setting the cluster setting `sql.stats.statement_fingerprint.format_mask`
to include `FmtCollapseLists`.

```
FmtHideConstants (previous behaviour):
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, _), (_, _)), ((_, _), (_, _)), ((_, _), (_, _))

ARRAY[1, 2, 3+4] ->
ARRAY[_, _, _+4]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, 2, __more1_10__)

---

FmtCollapseLists:
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, __more__), __more__), (__more__)

ARRAY[1, 2, 3+4] ->
ARRAY[_, __more__]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, __more__)
```

Epic: none
Part of: cockroachdb#120409

Release note: none
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 15, 2024
The new AST format flag  `FmtConstantsAsUnderscores` can be used
to format placeholders and literals as the same char to reduce any potential
cardinality from variances in formatting string literals, numbers, and
placeholders. FmtConstantsAsUnderscores will replace all placeholders, literals,
and string literals with `_`.

For example:
```
FmtHideConstants:
UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = $1

UPDATE foo SET a = '1', b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = '_', b = _, c = _ WHERE d = $1

UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = 5 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = _

FmtConstantsAsUnderscores will produce the following for all 3:
UPDATE foo SET a = _, b = _, c = _ WHERE d = _
```

Epic: none
Part of: cockroachdb#120409

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 18, 2024
…cial flags

By default `sql.stats.statement_fingerprint.format_mask` is now
set to `FmtCollapseLists|FmtConstantsAsUnderscores` to reduce
statement fingerprint cardinality due to long constant lists and
variations in constant formatting.

Closes: cockroachdb#120409

Release note (sql change): Users will see the following changes
in their generated statement fingerprints from sql stats:
- lists with only literals/placeholders and similar subexpressions are
shortened to their first item followed by "__more__", e.g.
```
SELECT * FROM foo WHERE f IN (1, $1, 1+2) ->
SELECT * FROM foo WHERE f IN (_, __more__)
```
- constants and placeholders are all replaced with the same character,
an underscore `_`
craig bot pushed a commit that referenced this issue Mar 19, 2024
120410: sql: allow additional fmt flags for stmt fingerprinting r=xinhaoz a=xinhaoz

This commit introduces a way to add additional format flags when
formatting a statement AST into its statement fingerprint representation
for sql stats. This allows us to more aggressively generalize the
statement fingerprint.

`sql.stats.statement_fingerprint.format_mask` will be used to supply
these additional flags to the formatter.  It is currently 0 by default
since no new flags for fingerprints have been added.

Epic: none
Part of: #120409

Release note: None

120646: telemetryccl: use log spy in backup/restore test r=abarganier a=dhartunian

Previously this test used file logging to test the telemetry output, which can result in flakes on CI. This commit modifies the test to use a log spy which is a bit more reliable. Additionally, the deserialization now happens in the `Intercept()` method which makes the test easier to read.

Resolves: #120115
Epic: None
Release note: None

120653: server: refactor TestAdminDebugRedirect test r=abarganier a=dhartunian

Adjusts test to use more standard redirect ignoring behavior in stdlib, and removes the test tenant override since this test works with tenants now after some adjustments to URL handling.

The #120095 issue was a timeout that this change doesn't explicitly deal with here since that problem isn't reproducible. The hope is that modified redirect error handling might trigger a less error-prone branch in the HTTP-client. There's nothing else to really change in this test since it's quite simple and we haven't seen similar timeouts persist in other HTTP tests.

Resolves: #120095
Resolves: #112955
Epic: None

Release note: None

120699: sql: skip TestSqlActivityUpdateTopLimitJob r=abarganier a=dhartunian

Release note: None

120715: workflows: tag `cockroach` builds for integration tests r=rail a=rickystewart

... with the tag `integration-test-artifact-build`. We do this to track how long it takes to build these artifacts specifically.

Epic: CRDB-8308
Release note: None

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: David Hartunian <davidh@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 20, 2024
Add `FmtCollapseLists` flag to shorten long lists to a representative
two element list where the second element is simply `__more__` to
represent the rest of the list.

Specifically, where `FmtHideConstants` would shorten tuples, arrays and
VALUES clauses to its first two elements if it coontains only literals
and placeholders, `FmtCollapseLists` shortens these lists to its first
element if all items in the list are any combination of placeholders,
literals, or a simple subexpressions containing only literals and/or
placeholders.

Enabling this behaviour for statement fingerprint generation can be done
via setting the cluster setting `sql.stats.statement_fingerprint.format_mask`
to include `FmtCollapseLists`.

```
FmtHideConstants (previous behaviour):
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, _), (_, _)), (__more1_10__)

SELECT * FROM foo WHERE f IN ((1, 2), (3, 4), (5, 6)) ->
SELECT * FROM foo WHERE f IN ((_, _), (_, _), (_, _))

ARRAY[1, 2, 3+4] ->
ARRAY[_, _, _+_]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, 2, __more1_10__)

---

FmtHideConstants | FmtCollapseLists:
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, __more__), __more__), (__more__)

SELECT * FROM foo WHERE f IN ((1, 2), (3, 4), (5, 6)) ->
SELECT * FROM foo WHERE f IN ((_, __more__), __more__)

ARRAY[1, 2, 3+4] ->
ARRAY[_, __more__]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, __more__)
```

Epic: none
Part of: cockroachdb#120409

Release note: none
craig bot pushed a commit that referenced this issue Mar 20, 2024
120023: roachtest: add run-operation command to run singular operation  r=HerkoLategan a=itsbilal

NB: This change builds on #119796. Review that first.

This change adds a new roachtest command, run-operation,
that runs a singular operation defined in OperationSpec
(see #119796), in a simplified harness, with existing
clusters only. The existing cluster is not touched (eg.
stopped or wiped), unless the operation explicitly
does an action like that.

An implementation of operation.Operation is also bundled in this
change to make these operations runnable.

Epic: none

Release note: None

120430: sql/sem/tree: add FmtForFingerprint functions to collapse long lists  r=xinhaoz a=xinhaoz

Please note only the latest commit should be reviewed here.

---------------
Add functions for the `FmtForFingerprint` flag to shorten long lists
to a representative two element list where the second element is simply
`__more__` to represent the rest of the list.

Specifically, where `FmtHideConstants` would shorten tuples, arrays and
VALUES clauses to its first two elements if it coontains only literals
and placeholders, `FmtCollapseLists` shortens these lists to its first
element if all items in the list are any combination of placeholders,
literals, or a simple subexpressions containing only literals and/or
placeholders.

Enabling this behaviour for statement fingerprint generation can be done
via setting the cluster setting `sql.stats.statement_fingerprint.format_mask`
to include `FmtCollapseLists`.

```
FmtHideConstants (previous behaviour):
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, _), (_, _)), ((_, _), (_, _)), ((_, _), (_, _))

ARRAY[1, 2, 3+4] ->
ARRAY[_, _, _+4]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, 2, __more1_10__)

---

FmtHideConstants | FmtCollapseLists:
VALUES ((1, 2), (3, 4)), ((5, 6), (7, 8)), ((9, 10), (11, 12)) ->
VALUES ((_, __more__), __more__), (__more__)

ARRAY[1, 2, 3+4] ->
ARRAY[_, __more__]

SELECT * FROM foo WHERE a IN (1, 2, 3, 4, 5) ->
SELECT * FROM foo WHERE a IN (1, __more__)
```


Epic: none
Part of: #120409
Release note: none

120594: ccl/serverccl: skip flakey assertion r=msbutler a=stevendanna

Informs #119329
Release note: None

120686: plpgsql: implement CONTINUE/EXIT with condition r=DrewKimball a=DrewKimball

PL/pgSQL allows adding a `WHEN <condition>` clause to `EXIT` and `CONTINUE` statements. This is syntactic sugar for `EXIT` or `CONTINUE` within an `IF` statement. This commit add support for this syntax.

Informs #115271

Release note (sql change): It is now possible to specify a condition for the PL/pgSQL statemenets `EXIT` and `CONTINUE`.

120760: roachprod: remove `admin-ui-port` flag from `start-sql` r=DarrylWong a=renatolabs

This flag is only applicable to the system tenant (`roachprod start`); having it on `start-sql` is misleading.

Epic: none

Release note: None

Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Renato Costa <renato@cockroachlabs.com>
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 20, 2024
The new AST format flag  `FmtConstantsAsUnderscores` can be used
to format placeholders and literals as the same char to reduce any potential
cardinality from variances in formatting string literals, numbers, and
placeholders. FmtConstantsAsUnderscores will replace all placeholders, literals,
and string literals with `_`.

For example:
```
FmtHideConstants:
UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = $1

UPDATE foo SET a = '1', b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = '_', b = _, c = _ WHERE d = $1

UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = 5 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = _

FmtConstantsAsUnderscores will produce the following for all 3:
UPDATE foo SET a = _, b = _, c = _ WHERE d = _
```

Epic: none
Part of: cockroachdb#120409

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 20, 2024
…cial flags

By default `sql.stats.statement_fingerprint.format_mask` is now
set to `FmtCollapseLists|FmtConstantsAsUnderscores` to reduce
statement fingerprint cardinality due to long constant lists and
variations in constant formatting. Note that the default fmt flag
for statement fingerprint generation is `FmtHideConstants`. Any
flags set with sql.stats.statement_fingerprint.format_mask will be
OR'd with `FmtHideConstants`.

Closes: cockroachdb#120409

Release note (sql change): Users will see the following changes
in their generated statement fingerprints from sql stats:
- lists with only literals/placeholders and similar subexpressions are
shortened to their first item followed by "__more__", e.g.
- constants and placeholders are all replaced with the same character,
an underscore `_`
```
SELECT * FROM foo WHERE f IN (1, $1, 1+2) ->
SELECT * FROM foo WHERE f IN (_, __more__)
```
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 20, 2024
The new AST format flag  `FmtConstantsAsUnderscores` can be used
to format placeholders and literals as the same char to reduce any potential
cardinality from variances in formatting string literals, numbers, and
placeholders. FmtConstantsAsUnderscores will replace all placeholders, literals,
and string literals with `_`.

For example:
```
FmtHideConstants:
UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = $1

UPDATE foo SET a = '1', b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = '_', b = _, c = _ WHERE d = $1

UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = 5 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = _

FmtConstantsAsUnderscores will produce the following for all 3:
UPDATE foo SET a = _, b = _, c = _ WHERE d = _
```

Epic: none
Part of: cockroachdb#120409

Release note: None
craig bot pushed a commit that referenced this issue Mar 20, 2024
120496: pkg/sql/sem/tree: add FmtConstantsAsUnderscores fmt flag  r=xinhaoz a=xinhaoz

Please note that only the latest commit should be reviewed here.

---------------------------
The new AST format flag  `FmtConstantsAsUnderscores` can be used
to format placeholders and literals as the same char to reduce any potential
cardinality from variances in formatting string literals, numbers, and
placeholders. FmtConstantsAsUnderscores will replace all placeholders, literals,
and string literals with `_`.

For example:
```
FmtHideConstants:
UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = $1

UPDATE foo SET a = '1', b = 2, c = 3 WHERE d = $1 ->
UPDATE foo SET a = '_', b = _, c = _ WHERE d = $1

UPDATE foo SET a = 1, b = 2, c = 3 WHERE d = 5 ->
UPDATE foo SET a = _, b = _, c = _ WHERE d = _

FmtConstantsAsUnderscores will produce the following for all 3:
UPDATE foo SET a = _, b = _, c = _ WHERE d = _
```

Epic: none
Part of: #120409

120720: authors: add kevin.cao to authors r=kev-cao a=kev-cao

Release note: None
Epic: None

120764: util/mon: lose more references on Stop r=yuzefovich a=yuzefovich

This commit makes it so that we also now lose the references from the monitor being `Stop`ped to its siblings to aid GC. This was omitted by mistake originally and _perhaps_ could make GC's job harder.

Epic: None

Release note: None

120765: Revert "flowinfra: fix incomplete shutdown in some cases" r=yuzefovich a=yuzefovich

This reverts commit f931435.

This commit made the multitenant-upgrade roachtest fail intermittently, so let's revert it for now (I plan to look more closely during the stability period).

Informs: #119065

Epic: None

Release note: None

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Kevin Cao <kevin.cao@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 21, 2024
…cial flags

By default `sql.stats.statement_fingerprint.format_mask` is now
set to `FmtCollapseLists|FmtConstantsAsUnderscores` to reduce
statement fingerprint cardinality due to long constant lists and
variations in constant formatting. Note that the default fmt flag
for statement fingerprint generation is `FmtHideConstants`. Any
flags set with sql.stats.statement_fingerprint.format_mask will be
OR'd with `FmtHideConstants`.

Closes: cockroachdb#120409

Release note (sql change): Users will see the following changes
in their generated statement fingerprints from sql stats:
- lists with only literals/placeholders and similar subexpressions are
shortened to their first item followed by "__more__", e.g.
- constants and placeholders are all replaced with the same character,
an underscore `_`
```
SELECT * FROM foo WHERE f IN (1, $1, 1+2) ->
SELECT * FROM foo WHERE f IN (_, __more__)
```
craig bot pushed a commit that referenced this issue Mar 22, 2024
120507: sql: default sql.stats.statement_fingerprint.format_mask to use special flags r=xinhaoz a=xinhaoz

Please note only the latest commit should be reviewed.

------------------------
By default `sql.stats.statement_fingerprint.format_mask` is now
set to `FmtCollapseLists|FmtConstantsAsUnderscores` to reduce
statement fingerprint cardinality due to long constant lists and
variations in constant formatting. Note that the default fmt flag
for statement fingerprint generation is `FmtHideConstants`. Any
flags set with sql.stats.statement_fingerprint.format_mask will be
OR'd with `FmtHideConstants`.

Closes: #120409

Release note (sql change): Users will see the following changes
in their generated statement fingerprints from sql stats:
- lists with only literals/placeholders and similar subexpressions are
shortened to their first item followed by "__more__", e.g.
- constants and placeholders are all replaced with the same character,
an underscore `_`
```
SELECT * FROM foo WHERE f IN (1, $1, 1+2) ->
SELECT * FROM foo WHERE f IN (_, __more__)
```

120596: kvcoord: add observability for DistSender circuit breakers r=erikgrinaker a=erikgrinaker

**util/circuit: add `error` parameter for `EventHandler.OnReset`**

This allows the `OnReset` handler to determine whether the circuit breaker was tripped when reset. This can happen e.g. if an async probe succeeds before the breaker has been tripped.

**kvcoord: add DistSender circuit breaker metrics**

This patch adds an initial set of DistSender circuit breaker metrics:

* distsender.circuit_breaker.replicas.count
* distsender.circuit_breaker.replicas.tripped
* distsender.circuit_breaker.replicas.tripped_events
* distsender.circuit_breaker.replicas.probes.running
* distsender.circuit_breaker.replicas.probes.success
* distsender.circuit_breaker.replicas.probes.failure
* distsender.circuit_breaker.replicas.requests.cancelled
* distsender.circuit_breaker.replicas.requests.rejected

**kvcoord: annotate DistSender circuit breaker async contexts**

**kvcoord: improve DistSender circuit breaker errors**

**kvcoord: add logging/tracing for DistSender circuit breakers**

By default, only the trip/reset events are logged. vmodule logs yield:

```
I240316 13:07:33.815949 113 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:899  [T1,Vsystem,n1] 243  launching circuit breaker probe for r68/(n3,s3):3 (tripped=false stall=1.318s error=0s)
I240316 13:07:33.816084 4941 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:811  [T1,Vsystem,n1] 244  sending probe to r68/(n3,s3):3: LeaseInfo [/Table/Max]
I240316 13:07:34.816556 4941 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:813  [T1,Vsystem,n1] 249  probe result from r68/(n3,s3):3: br=<nil> err=ba: LeaseInfo [/Table/Max] RPC error: grpc: context deadline exceeded [code 4/DeadlineExceeded]
E240316 13:07:34.816788 4941 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:873  [T1,Vsystem,n1] 250  r68/(n3,s3):3 circuit breaker tripped: probe timed out: context deadline exceede  (stalled for 2.32s, erroring for 0s)
E240316 13:07:38.023324 5287 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:519  [T1,Vsystem,n1] 535  request rejected by tripped circuit breaker for r68/(n3,s3):3: probe timed out: context deadline exceeded
I240316 13:07:38.024899 9324 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:813  [T1,Vsystem,n1] 550  probe result from r68/(n3,s3):3: br=<nil> err=ba: LeaseInfo [/Table/Max] RPC error: grpc: context canceled [code 1/Canceled]
I240316 13:07:38.025341 9324 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:890  [T1,Vsystem,n1] 556  r68/(n3,s3):3 circuit breaker reset
I240316 13:07:38.025374 9324 kv/kvclient/kvcoord/dist_sender_circuit_breaker.go:906  [T1,Vsystem,n1] 557  stopping circuit breaker probe for r68/(n3,s3):3 (tripped=false lastRequest=1.344s)
```

Resolves #119916.
Epic: none
Release note: None

**kvcoord: use duration helpers in DistSender circuit breakers**

**kvcoord: change DistSender circuit breaker string identifier**

Changes e.g. `r68/(n3,s3):3` to `r68/3:(n3,s3)`.

120908: go.mod: bump Pebble to 10ebcdd794ec r=itsbilal a=aadityasondhi

Changes:

 * [`10ebcdd7`](cockroachdb/pebble@10ebcdd7) metamorphic: track IngestAndExcise in keymgr and resolve singledel conflicts
 * [`b3c1664a`](cockroachdb/pebble@b3c1664a) db: fix nil map error when ingest-splitting during flushable ingests
 * [`fd5dc141`](cockroachdb/pebble@fd5dc141) metamorphic: re-enable ingest split and ingestAndExcise in TestMeta
 * [`4335ae09`](cockroachdb/pebble@4335ae09) keyspan: simplify Filter
 * [`30f455fa`](cockroachdb/pebble@30f455fa) keyspan: simplify and reimplement Truncate
 * [`e2f53d2d`](cockroachdb/pebble@e2f53d2d) wal: fix recordQueue bug due to forgetting to mod when indexing
 * [`8cdabcc9`](cockroachdb/pebble@8cdabcc9) metamorphic: add support for external ingestions in replicateOp
 * [`e5c9f635`](cockroachdb/pebble@e5c9f635) db: remove strictWALTail option
 * [`3c9893d6`](cockroachdb/pebble@3c9893d6) ingest: fix ingestion metric for flushableIngest
 * [`8a097e8a`](cockroachdb/pebble@8a097e8a) ingest,compaction: use excise when flushing flushableIngest
 * [`dc7ccb2b`](cockroachdb/pebble@dc7ccb2b) vfs/vfstest: adjust WithOpenFileTracking to not wrap typed nils
 * [`c268820f`](cockroachdb/pebble@c268820f) meta: temporarily disable downloadOp
 * [`2aa3786c`](cockroachdb/pebble@2aa3786c) sstable: split block.go
 * [`702f8cc3`](cockroachdb/pebble@702f8cc3) base: add UserKeyBounds

Release note: none.
Epic: none.

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Co-authored-by: Aaditya Sondhi <20070511+aadityasondhi@users.noreply.github.com>
@craig craig bot closed this as completed in bf24702 Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-observability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant