Previously, when attempting to get a stmt or txn entry
in the sql stats containers, we pass a flag to the get*
functions specifying whether or not to create the entry if
it does not exist. This commit cleans up this interface by
splitting the function up into a pure get or tryCreate functions.

Note that originally we were pursuing a double checked lock
approach, however Golang's RWMutex does not scale well with
the number of CPUs. Instead this patch just simplifies the
code here a bit to make some incoming changes that will eventually
move this stats recording step out of the execution path easier.
Once that happens we can investigate further improvment or
removal of these mutexes.

Epic: none
Release note: None
Part of: cockroachdb#141024

140604: roachtest: use atomic pointer for logger r=srosenberg,herkolategan a=DarrylWong

The test runner swaps out the test logger when running post test artifacts collection and checks. However, in the case of a timeout, the test goroutine may still be running and have access to the test logger. This leads to a race condition where the logger is replaced as it's being used by the test.

This change switches the test logger to use an atomic pointer instead.

Fixes: none
Epic: none
Release note: none

141044: physicalplan: minor fixes to pooling of FlowSpecs r=yuzefovich a=yuzefovich

**physicalplan: ensure that FlowSpecs are released**

We pool `FlowSpec` allocations in `GenerateFlowSpecs`, but previously we would only release them back to the pool on the main query path. This commit fixes that oversight.

**physicalplan: fix minor leak with pooling of FlowSpecs**

The only thing that we reuse when pooling `FlowSpec` objects is the capacity of `Processors` slice. Previously, we forgot to unset each element of that slice, so we could have hold onto to some processor specs which in turn could have hold onto some large objects (like `roachpb.Span`s in the TableReader) until a particular index of the slice is overwritten. This oversight is now fixed.

Found while looking at #140326.
Epic: None
Release note: None

141045: sqlstats: mutex improvements r=xinhaoz a=xinhaoz

Previously, when attempting to get a stmt or txn entry in the sql stats containers, we pass a flag to the get* functions specifying whether or not to create the entry if it does not exist. This commit cleans up this interface by splitting the function up into a pure get or tryCreate functions.

Note that originally we were pursuing a double checked lock approach, however Golang's RWMutex does not scale well with the number of CPUs. Instead this patch just simplifies the code here a bit to make some incoming changes that will eventually move this stats recording step out of the execution path easier. Once that happens we can investigate further improvment or removal of these mutexes.

Epic: none
Release note: None
Part of: #141024

### sqlstats: convert RWMutex to Mutex

This commit changes the RWMutex on the sql stats
containers to the regular exclusive Mutex struct.
The motivation behind this change is that RWMutex
scales poorly with CPU count. See benchmarks below.

Epic: none
Part of: #140590

141146: sqlstats: delete unused iterators r=kyle-a-wong a=kyle-a-wong

deletes unused iterators in persistedsqlstats

Epic: None
Release note: None

Co-authored-by: DarrylWong <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Kyle Wong <[email protected]>

… container

Previously, each stats collector for a sql session  would buffer a
transaciton's stmt stats in a ssmemstorage.Container struct which has
a lot of additional overhead when reading and writing as it manages
synchronization logic. For this per-session writing, this writing
is synchronous so there's no need to use such a container. We should
just buffer the stmts in a simple slice. Essentially, this shouldn't
make anything worse as we're just simplifying the buffer.

Note that this is simply a temporary simplified state. Eventually as
part of cockroachdb#141024 we will move the writing to the shared container to be
async from query execution.

Epic: none

Release note: None

This commit adds the `FingerprintID` field to sqlstats.RecordedTxnStats.

Epic: none
Part of: cockroachdb#141024

Release note: None

This commit adds the `FingerprintID` field to sqlstats.RecordedTxnStats.

Epic: none
Part of: cockroachdb#141024

Release note: None

This commit adds the `FingerprintID` field to sqlstats.RecordedTxnStats.

Epic: none
Part of: cockroachdb#141024

Release note: None

143016: sslocal: stats collector cleanup r=xinhaoz a=xinhaoz

Commit 1
----------
### sqlstats: reduce stats collector calls from conn exec
Previously, separate functions in the stats collector were required to
send execution statistics for statements and transactions to sqlstats
and insights respectively.

This commit consolidates the sending of execution events to sqlstats
and insights into one function on the stats collector per event type
(one for statements and one for transactions).

Epic: none
Part of: #141024
Release note: None


Commit 2
----------
### sqlstats: add FingerprintID to sqlstats.RecordedTxnStats

This commit adds the `FingerprintID` field to sqlstats.RecordedTxnStats.
Epic: none
Part of: #141024

Release note: None

Commit 3
----------
### sqlstats,insights: add struct conversion functions
This commit adds functions converting sqlstats.Recorded{Stmt,Txn}Stats
to `insight.{Statement,Transaction}`.
Next: insights will only create insight objects if we are putting them
into the insights cache, after analyzing `sqlstats.Recorded*` objects.

Epic: none

Release note: None

Co-authored-by: Xin Hao Zhang <[email protected]>

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

For reviewers: this commit mostly deals with changing logic in the insights
package and related testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats`
instead of `insights.{Statement,Transaction}` types. We also change the function
signatures for the sqlstats containers to accept pointers to `sqlstats.Recorded*`
objects instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

For reviewers: this commit mostly deals with changing logic in the insights
package and related testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats`
instead of `insights.{Statement,Transaction}` types. We also change the function
signatures for the sqlstats containers to accept pointers to `sqlstats.Recorded*`
objects instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

This commit mostly deals with changing logic in the insights package and related
testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats` instead of
`insights.{Statement,Transaction}` types. We also change the function signatures
for the sqlstats containers to accept pointers to `sqlstats.Recorded*` objects
instead of by value.

Epic: none
Part of: cockroachdb#141024
Release note: None

141767: insights: insights and sqlstats subsystem should consume the same types r=xinhaoz a=xinhaoz

Previously, insights consumed its own data type containing per-execution
statisitcs about statements and transactions for outlier analysis. These
should only be created if we determine that the execution should be cached
in our insights registry.

SQL stats and insights will now consume the same data types to process and
perform their respective recordings: `sqlstats.RecordedStmtStats` and
`sqlstats.RecordedTxnStats`, which contain per execution statistics. This
is supporting work in moving sql stats recording to to occur in the same
async routine as insights.

For reviewers: this commit mostly deals with changing logic in the insights
package and related testing to perform analysis on `sqlstats.Recorded{Stmt,Txn}Stats`
instead of `insights.{Statement,Transaction}` types. We also change the function
signatures for the sqlstats containers to accept pointers to `sqlstats.Recorded*`
objects instead of by value.

Epic: none
Part of: #141024
Release note: None

Co-authored-by: Xin Hao Zhang <[email protected]>

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit removes the recording of sql stats at the end of statement and
transaction execution. The per-node SQLStats container has been added as a
sink to the async sql stats ingester. Instead of having sslocal.StatsCollector
record sql stats at the end of execution, it will now record the stats
asynchronously as part of event processing in sslocal.SQLStatsIngester's
ingest routine.

Epic: none
Closes: cockroachdb#141024

Release note: None

This commit removes the recording of sql stats at the end of statement and
transaction execution. The per-node SQLStats container has been added as a
sink to the async sql stats ingester. Instead of having sslocal.StatsCollector
record sql stats at the end of execution, it will now record the stats
asynchronously as part of event processing in sslocal.SQLStatsIngester's
ingest routine.

Epic: none
Closes: cockroachdb#141024

Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: cockroachdb#141024

Epic: none
Release note: None

143669: sql: add timeout for PCR reader catalog lease acquisition r=fqazi a=fqazi

Previously, the logic to determine if a PCR reader catalog was in use could become stuck if an availability issue occurred with the leasing subsystem. This was because we could end up waiting indefinitely for the lease in failure scenarios like TestUnavailableZipDir, and the statement_timeout is not active this early. To address this, this patch adds a 30-second timeout for obtaining a lease on the system database when detecting PCR reader catalogs.

Fixes: #141565

Release note: None

143768: sqlstats: generalize insight.ConcurrentBufferIngester  r=xinhaoz a=xinhaoz

This commit copies the ConcurrentBufferIngester from insights into the sqlstats
pkg in preparation to have it consume the sql execution event and handle the
writing to both sqlstats and insights systems, instead of just for insights. At
this stage we are preserving how this ingester consumes events. The ingester is
still only used for insights writing.

Previously, the insights registry had the responsibility of buffering statements
by session id and processing them once the entire transaction completes (via
receicing a transaction event). The responsibility of buffering statements by
session has now moved to the ingester. As a result we remove the
`ObserveStatement` function from the insights registry which was only used to
insert into the statement buffer.

Part of: #141024

Epic: none
Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>

This commit removes the recording of sql stats at the end of statement and
transaction execution. The per-node SQLStats container has been added as a
sink to the async sql stats ingester. Instead of having sslocal.StatsCollector
record sql stats at the end of execution, it will now record the stats
asynchronously as part of event processing in sslocal.SQLStatsIngester's
ingest routine.

Epic: none
Closes: cockroachdb#141024

Release note: None