Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…79409 #79427 #79428 #79433 #79444

76312: kvserver, batcheval: pin Engine state during read-only command evaluation r=aayushshah15 a=aayushshah15

This commit makes it such that we eagerly pin the engine state of the `Reader`
created during the evaluation of read-only requests.

Generally, reads will hold latches throughout the course of their evaluation
(particularly, while they do their `MVCCScan`). Mainly, this commit paves the
way for us to move to a world where we avoid holding latches during the
MVCCScan. Additionally, it also lets us make MVCC garbage collection latchless
as described in #55293.

There are a few notable changes in this patch:

1. Pinning the engine state eagerly runs into #70974. To resolve this, the
closed timestamp of the `Replica` is now captured at the time the `EvalContext`
is created, and not during the command evaluation of
`QueryResolvedTimestampRequest`.

2. `EvalContext` now has a `ImmutableEvalContext` embedded into it. The
`ImmutableEvalContext` is supposed to encapsulate state that must not change
after the `EvalContext` is created. The closed timestamp of the replica is part
of the `ImmutableEvalContext`.

3. `Replica` no longer fully implements the `EvalContext` interface. Instead,
it implements everything but `GetClosedTimestamp()` (which is implemented by
`ImmutableEvalContext` instead).

Relates to #55293
Resolves #55461
Resolves #70974

Release note: None


78652: sql: implement to_reg* builtins r=otan a=e-mbrown

Resolves #77838
This commit implements the `to_regclass`, `to_regnamespace`, `to_regproc`,
`to_regprocedure`, `to_regrole`, and `to_regtype` builtins.

Release note (<category, see below>): The `to_regclass`, `to_regnamespace`, `to_regproc`,
`to_regprocedure`, `to_regrole`, and `to_regtype` builtin functions are now supported,
improving compatibility with PostgreSQL.

79022: server/status: add running non-idle jobs metric r=darinpp a=darinpp

Previously serverless was using the sql jobs running metric to determine
if a tenant process is idle and can be shut down. With the introduction
of continiously running jobs this isn't a good indicator anymore. A
recent addition is a per job metrics that show running or idle. The auto
scaler doesn't care about the individual jobs and only cares about the
total number of jobs that a running but haven't reported as being idle.
The pull rate is also very high so the retriving all the individual
running/idle metrics for each job type isn't optimal. So this PR adds a
single metric that just aggregates and tracks the total count of jobs
running and not idle.

Release justification: Bug fixes and low-risk updates to new functionality
Release note: None

Will be re-based once #79021 is merged

79157: cli: tweak slow decommission message r=knz a=cameronnunez

Release note: None

79313: opt: do not push LIMIT into the scan of a virtual table r=msirek a=msirek

Fixes #78578

Previously, a LIMIT operation could be pushed into the scan of a virtual
table with an ORDER BY clause.              

This was inadequate because in-order scans of virtual indexes aren't
supported. When an index that should provide the order requested by a
query is used, a sort is actually produced under the covers:
```
EXPLAIN(vec)
SELECT oid, typname FROM pg_type ORDER BY OID;
               info
----------------------------------
  │
  └ Node 1
    └ *colexec.sortOp
      └ *sql.planNodeToRowSource

```
Functions `CanLimitFilteredScan` and `GenerateLimitedScans` are modified
to avoid pushing LIMIT operations into ordered scans of virtual indexes. 

Release justification: Low risk fix for incorrect results in queries
involving virtual system tables.

Release note (bug fix): LIMIT queries with an ORDER BY clause which scan
the index of a virtual system tables, such as `pg_type`, could
previously return incorrect results. This is corrected by teaching the
optimizer that LIMIT operations cannot be pushed into ordered scans of
virtual indexes.


79346: ccl/sqlproxyccl: add rebalancer queue for connection rebalancing r=JeffSwenson a=jaylim-crl

#### ccl/sqlproxyccl: add rebalancer queue for rebalance requests 

This commit adds a rebalancer queue implementation to the balancer component.
The queue will be used for rebalance requests for the connection migration
work. This is done to ensure a centralized location that invokes the
TransferConnection method on the connection handles. Doing this also enables
us to limit the number of concurrent transfers within the proxy.

Release note: None

#### ccl/sqlproxyccl: run rebalancer queue processor in the background 

The previous commit added a rebalancer queue. This commit connects the queue to
the balancer, and runs the queue processor in the background. By the default,
we limit up to 100 concurrent transfers at any point in time, and each transfer
will be retried up to 3 times.

Release note: None

Jira issue: CRDB-14727

79362: kv: remove stale comment in processOneChange r=nvanbenschoten a=nvanbenschoten

The comment was added in 2fb56bd and hasn't been accurate since 5178559.

Jira issue: CRDB-14753

79368: ccl/sqlproxyccl: include DRAINING pods in the directory cache r=JeffSwenson a=jaylim-crl

Previously, #67452 removed DRAINING pods from the directory cache. This commit
adds that back. The connector will now need to filter for RUNNING pods manually
before invoking the balancer. This is needed so that we could track DRAINING
pods, and wait until 60 seconds has elapsed before transferring connections
away from them. To support that, we also update the Pod's proto definition to
include a StateTimestamp field to reprevent that timestamp that the state field
was last updated.

The plan is to have a polling mechanism every X seconds to check DRAINING pods,
and use that information to start migrating connections.

Release note: None

Jira issue: CRDB-14759

79386: colexec: remove redundant benchmarks r=yuzefovich a=yuzefovich

This commit finishes the transition of some of the benchmarks in the
colexec package started in 22.1 cycle.

Fixes: #75106.

Release note: None

Jira issue: CRDB-14783

79409: sql: refactor deps tests to use bazel r=yuzefovich a=yuzefovich

This commit refactors most `VerifyNoImports` dependency tests in the sql
folder to use the newly introduced bazel test utilities.

Release note: None

Jira issue: CRDB-14814

79427: backupccl: allow cluster restore from different tenant r=dt a=stevendanna

This removes a prohibition for cluster restores with mismatched tenant
IDs since we believe they are now correct as of #73831

This allows users to take a cluster backup in a tenant and restore it
into another tenant.

The new tenant_settings table needs special care since it may exist in
the source tenant but not the target tenant when the source tenant is
the system tenant.

In this change, we throw an error in the case of a non-empty
tenant_settings table being restored into a non-system tenant. This is
a bit user-unfriendly since we detect this error rather late in the
restore process.

Release note: None

Jira issue: CRDB-14844

79428: backupccl: Refactor encryption utility functions into their own file. r=benbardin a=benbardin

Release note: None

Jira issue: CRDB-14845

79433: sql: use new ALTER TENANT syntax in tests r=stetvendanna a=rafiss

Release note: None

79444: roachtest: warmup follower-reads for fixed duration, not fixed number of ops r=nvanbenschoten a=nvanbenschoten

Fixes #78596.

This change switches the warmup phase of the follower-read roachtest
suite from running a fixed number of operations (100) to running for a
fixed duration (15s). This should ensure that the single-region variant
of the test is given sufficient time to warm up follower reads immediately
after one of its nodes is restarted.

Before this change, the single-region variant was only being given about
500ms after startup to catch up on the closed timestamp, which made the
test flaky.

Release justification: testing only

Co-authored-by: Aayush Shah <[email protected]>
Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: Darin Peshev <[email protected]>
Co-authored-by: Cameron Nunez <[email protected]>
Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: Jay <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Ben Bardin <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
12 people committed Apr 5, 2022
15 parents 4f275c3 + aaa9cac + f6048b2 + 09e5af0 + a21eab1 + 1d82da6 + 02b5be6 + af64a4a + 7586717 + 54bdfd1 + f72386c + 46c04e2 + 8c9fc62 + 3697283 + 5192665 commit 985344a
Show file tree
Hide file tree
Showing 113 changed files with 2,166 additions and 1,566 deletions.
12 changes: 12 additions & 0 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -3072,6 +3072,18 @@ table. Returns an error if validation fails.</p>
</span></td></tr>
<tr><td><a name="session_user"></a><code>session_user() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the session user. This function is provided for compatibility with PostgreSQL.</p>
</span></td></tr>
<tr><td><a name="to_regclass"></a><code>to_regclass(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual relation name to its OID</p>
</span></td></tr>
<tr><td><a name="to_regnamespace"></a><code>to_regnamespace(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual schema name to its OID</p>
</span></td></tr>
<tr><td><a name="to_regproc"></a><code>to_regproc(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual function or procedure name to its OID</p>
</span></td></tr>
<tr><td><a name="to_regprocedure"></a><code>to_regprocedure(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual function or procedure name(with argument types) to its OID</p>
</span></td></tr>
<tr><td><a name="to_regrole"></a><code>to_regrole(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual role name to its OID</p>
</span></td></tr>
<tr><td><a name="to_regtype"></a><code>to_regtype(text: <a href="string.html">string</a>) &rarr; regtype</code></td><td><span class="funcdesc"><p>Translates a textual type name to its OID</p>
</span></td></tr>
<tr><td><a name="version"></a><code>version() &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the node’s version of CockroachDB.</p>
</span></td></tr></tbody>
</table>
Expand Down
24 changes: 21 additions & 3 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ ALL_TESTS = [
"//pkg/cmd/roachtest:roachtest_test",
"//pkg/cmd/teamcity-trigger:teamcity-trigger_test",
"//pkg/cmd/testfilter:testfilter_test",
"//pkg/col/coldata:coldata_disallowed_imports_test",
"//pkg/col/coldata:coldata_test",
"//pkg/col/coldataext:coldataext_test",
"//pkg/col/colserde:colserde_test",
Expand Down Expand Up @@ -235,25 +236,34 @@ ALL_TESTS = [
"//pkg/sql/colconv:colconv_test",
"//pkg/sql/colencoding:colencoding_test",
"//pkg/sql/colexec/colbuilder:colbuilder_test",
"//pkg/sql/colexec/colexecagg:colexecagg_test",
"//pkg/sql/colexec/colexecagg:colexecagg_disallowed_imports_test",
"//pkg/sql/colexec/colexecargs:colexecargs_test",
"//pkg/sql/colexec/colexecbase:colexecbase_disallowed_imports_test",
"//pkg/sql/colexec/colexecbase:colexecbase_test",
"//pkg/sql/colexec/colexeccmp:colexeccmp_test",
"//pkg/sql/colexec/colexechash:colexechash_disallowed_imports_test",
"//pkg/sql/colexec/colexechash:colexechash_test",
"//pkg/sql/colexec/colexecjoin:colexecjoin_disallowed_imports_test",
"//pkg/sql/colexec/colexecjoin:colexecjoin_test",
"//pkg/sql/colexec/colexecproj:colexecproj_disallowed_imports_test",
"//pkg/sql/colexec/colexecproj:colexecproj_test",
"//pkg/sql/colexec/colexecsel:colexecsel_disallowed_imports_test",
"//pkg/sql/colexec/colexecsel:colexecsel_test",
"//pkg/sql/colexec/colexecspan:colexecspan_disallowed_imports_test",
"//pkg/sql/colexec/colexecspan:colexecspan_test",
"//pkg/sql/colexec/colexectestutils:colexectestutils_test",
"//pkg/sql/colexec/colexecutils:colexecutils_test",
"//pkg/sql/colexec/colexecwindow:colexecwindow_disallowed_imports_test",
"//pkg/sql/colexec/colexecwindow:colexecwindow_test",
"//pkg/sql/colexec/execgen/cmd/execgen:execgen_test",
"//pkg/sql/colexec/execgen/cmd/execgen:execgen_disallowed_imports_test",
"//pkg/sql/colexec/execgen:execgen_test",
"//pkg/sql/colexec:colexec_disallowed_imports_test",
"//pkg/sql/colexec:colexec_test",
"//pkg/sql/colexecerror:colexecerror_test",
"//pkg/sql/colexecop:colexecop_test",
"//pkg/sql/colexecop:colexecop_disallowed_imports_test",
"//pkg/sql/colfetcher:colfetcher_test",
"//pkg/sql/colflow/colrpc:colrpc_test",
"//pkg/sql/colflow:colflow_disallowed_imports_test",
"//pkg/sql/colflow:colflow_test",
"//pkg/sql/colmem:colmem_test",
"//pkg/sql/contention/contentionutils:contentionutils_test",
Expand All @@ -265,18 +275,22 @@ ALL_TESTS = [
"//pkg/sql/distsql:distsql_test",
"//pkg/sql/doctor:doctor_test",
"//pkg/sql/enum:enum_test",
"//pkg/sql/execinfra:execinfra_disallowed_imports_test",
"//pkg/sql/execinfra:execinfra_test",
"//pkg/sql/execinfrapb:execinfrapb_disallowed_imports_test",
"//pkg/sql/execinfrapb:execinfrapb_test",
"//pkg/sql/execstats:execstats_test",
"//pkg/sql/flowinfra:flowinfra_disallowed_imports_test",
"//pkg/sql/flowinfra:flowinfra_test",
"//pkg/sql/gcjob/gcjobnotifier:gcjobnotifier_test",
"//pkg/sql/gcjob:gcjob_test",
"//pkg/sql/gcjob_test:gcjob_test_test",
"//pkg/sql/idxusage:idxusage_test",
"//pkg/sql/importer:importer_test",
"//pkg/sql/inverted:inverted_disallowed_imports_test",
"//pkg/sql/inverted:inverted_test",
"//pkg/sql/kvstreamer:kvstreamer_test",
"//pkg/sql/lex:lex_disallowed_imports_test",
"//pkg/sql/lex:lex_test",
"//pkg/sql/lexbase:lexbase_test",
"//pkg/sql/logictest:logictest_test",
Expand Down Expand Up @@ -326,7 +340,9 @@ ALL_TESTS = [
"//pkg/sql/rowenc/keyside:keyside_test",
"//pkg/sql/rowenc/valueside:valueside_test",
"//pkg/sql/rowenc:rowenc_test",
"//pkg/sql/rowexec:rowexec_disallowed_imports_test",
"//pkg/sql/rowexec:rowexec_test",
"//pkg/sql/rowflow:rowflow_disallowed_imports_test",
"//pkg/sql/rowflow:rowflow_test",
"//pkg/sql/scanner:scanner_test",
"//pkg/sql/scheduledlogging:scheduledlogging_test",
Expand Down Expand Up @@ -361,6 +377,7 @@ ALL_TESTS = [
"//pkg/sql/stmtdiagnostics:stmtdiagnostics_test",
"//pkg/sql/tests:tests_test",
"//pkg/sql/ttl/ttljob:ttljob_test",
"//pkg/sql/types:types_disallowed_imports_test",
"//pkg/sql/types:types_test",
"//pkg/sql:sql_test",
"//pkg/startupmigrations/leasemanager:leasemanager_test",
Expand Down Expand Up @@ -416,6 +433,7 @@ ALL_TESTS = [
"//pkg/util/interval/generic:generic_test",
"//pkg/util/interval:interval_test",
"//pkg/util/ipaddr:ipaddr_test",
"//pkg/util/json:json_disallowed_imports_test",
"//pkg/util/json:json_test",
"//pkg/util/limit:limit_test",
"//pkg/util/log/eventpb:eventpb_test",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ go_library(
"backup_processor_planning.go",
"backup_span_coverage.go",
"create_scheduled_backup.go",
"encryption.go",
"incrementals.go",
"key_rewriter.go",
"manifest_handling.go",
Expand Down Expand Up @@ -118,7 +119,6 @@ go_library(
"//pkg/util/contextutil",
"//pkg/util/ctxgroup",
"//pkg/util/encoding",
"//pkg/util/errorutil/unimplemented",
"//pkg/util/hlc",
"//pkg/util/interval",
"//pkg/util/ioctx",
Expand Down
17 changes: 0 additions & 17 deletions pkg/ccl/backupccl/alter_backup_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,16 @@
package backupccl

import (
"bytes"
"context"
"fmt"
"net/url"
"path"
"strings"

"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/featureflag"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -217,19 +213,6 @@ func doAlterBackupPlan(
return writeNewEncryptionInfoToBackup(ctx, encryptionInfo, baseStore, len(opts))
}

func writeNewEncryptionInfoToBackup(
ctx context.Context, opts *jobspb.EncryptionInfo, dest cloud.ExternalStorage, numFiles int,
) error {
// New encryption-info file name is in the format "ENCRYPTION-INFO-<version number>"
newEncryptionInfoFile := fmt.Sprintf("%s-%d", backupEncryptionInfoFile, numFiles+1)

buf, err := protoutil.Marshal(opts)
if err != nil {
return err
}
return cloud.WriteFile(ctx, dest, newEncryptionInfoFile, bytes.NewReader(buf))
}

func init() {
sql.AddPlanHook("alter backup", alterBackupPlanHook)
}
50 changes: 0 additions & 50 deletions pkg/ccl/backupccl/backup_destination.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"net/url"
"strings"

"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
Expand Down Expand Up @@ -251,55 +250,6 @@ func getBackupManifests(
return manifests, memMu.total, nil
}

// getEncryptionFromBase retrieves the encryption options of a base backup. It
// is expected that incremental backups use the same encryption options as the
// base backups.
func getEncryptionFromBase(
ctx context.Context,
user security.SQLUsername,
makeCloudStorage cloud.ExternalStorageFromURIFactory,
baseBackupURI string,
encryptionParams jobspb.BackupEncryptionOptions,
kmsEnv cloud.KMSEnv,
) (*jobspb.BackupEncryptionOptions, error) {
var encryptionOptions *jobspb.BackupEncryptionOptions
if encryptionParams.Mode != jobspb.EncryptionMode_None {
exportStore, err := makeCloudStorage(ctx, baseBackupURI, user)
if err != nil {
return nil, err
}
defer exportStore.Close()
opts, err := readEncryptionOptions(ctx, exportStore)
if err != nil {
return nil, err
}

switch encryptionParams.Mode {
case jobspb.EncryptionMode_Passphrase:
encryptionOptions = &jobspb.BackupEncryptionOptions{
Mode: jobspb.EncryptionMode_Passphrase,
Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrae), opts[0].Salt),
}
case jobspb.EncryptionMode_KMS:
var defaultKMSInfo *jobspb.BackupEncryptionOptions_KMSInfo
for _, encFile := range opts {
defaultKMSInfo, err = validateKMSURIsAgainstFullBackup(encryptionParams.RawKmsUris,
newEncryptedDataKeyMapFromProtoMap(encFile.EncryptedDataKeyByKMSMasterKeyID), kmsEnv)
if err == nil {
break
}
}
if err != nil {
return nil, err
}
encryptionOptions = &jobspb.BackupEncryptionOptions{
Mode: jobspb.EncryptionMode_KMS,
KMSInfo: defaultKMSInfo}
}
}
return encryptionOptions, nil
}

func readLatestFile(
ctx context.Context,
collectionURI string,
Expand Down
Loading

0 comments on commit 985344a

Please sign in to comment.