Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
101098: kvserver: acquire and switch leases during Raft ticks r=erikgrinaker a=erikgrinaker

**kvserver: acquire and switch leases during Raft ticks**

This patch eagerly acquires leases, and switches their type when appropriate, during Raft ticks. This avoids incurring lease acquisition latency on the next request to the range. However, it's only effective for unquiesced ranges (which includes all expiration-based ranges), so we retain the corresponding logic in the replicate queue as a fallback, taking effect within 10 minutes or so.

Resolves #98433.

Epic: none
Release note: None

**kvserver: eagerly initialize replicas with expiration leases**

By default, replicas start out quiesced and only unquiesce in response to range traffic. Since expiration-based leases should not quiesce, and should eagerly acquire leases, this patch eagerly initializes and unquiesces replicas with expiration-based leases when loaded.

Touches #98433.

Epic: none
Release note: None
  
**kvserver: limit concurrent eager lease acquisitions**

This patch limits the number of concurrent eager lease acquisitions done by the Raft scheduler to 256 per node (across all stores), configurable via `kv.lease.eager_acquisition_concurrency`. When the limit is reached, further lease acquisition requests are dropped and then retried on the next tick.

This only applies to acquisition of expired leases and switching of lease types done by the Raft scheduler, not to lease extensions nor to lease acquisitions driven by client requests. Since expiration leases need to be extended regularly there is little point in throttling them and the cluster must be provisioned to handle them, and client-driven acquisitions are clearly high priority.

A more sophisticated policy can be considered later if necessary.

Resolves #100426.

Epic: none
Release note: None

102516: backupccl: support materialized view mutation in restore r=rafiss a=rafiss

fixes #101075

Release note (bug fix): Fixed a bug that could prevent RESTORE from working if the backup had a refresh materialized view mutation in it.

103115: docgen: add USING clause to DELETE statement diagram r=yuzefovich a=taroface

Add the `USING` clause to the `DELETE` statement diagram. This was implemented in #40963.

Epic: none

Release note: none

Release justification: non-production code change

103143: pg_catalog: use 0 for pg_constraint.conparentid r=rafiss a=rafiss

fixes #103135

The Postgres docs say this is:
> The corresponding constraint of the parent partitioned table,
  if this is a constraint on a partition; else zero.

Since we don't support partitioning like Postgres, we should always make this zero.

This fixes a query that a tool was using to identify foreign key relationships.

Release note (bug fix): Stopped using a NULL value for pg_constraint.conparentid. Now the value is hard-coded to 0, since CockroachDB does not support constraints on partitions.

103217: roachtest: fix rust and npgsql blocklists r=rafiss a=rafiss

fixes #101724
fixes #102673
backport fixes #102740
backport fixes #101592

Release note: None

Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Ryan Kuo <[email protected]>
  • Loading branch information
4 people committed May 12, 2023
6 parents c72f25e + 5084670 + e4b74b8 + b2add06 + 1b9f3de + cd2db3e commit 117dc40
Show file tree
Hide file tree
Showing 19 changed files with 515 additions and 233 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/delete_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
delete_stmt ::=
( ( 'WITH' ( ( common_table_expr ) ( ( ',' common_table_expr ) )* ) | 'WITH' 'RECURSIVE' ( ( common_table_expr ) ( ( ',' common_table_expr ) )* ) ) | ) 'DELETE' 'FROM' ( ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) | ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) table_alias_name | ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) 'AS' table_alias_name ) opt_using_clause ( ( 'WHERE' a_expr ) | ) ( sort_clause | ) ( limit_clause | ) ( 'RETURNING' target_list | 'RETURNING' 'NOTHING' | )
( ( 'WITH' ( ( common_table_expr ) ( ( ',' common_table_expr ) )* ) | 'WITH' 'RECURSIVE' ( ( common_table_expr ) ( ( ',' common_table_expr ) )* ) ) | ) 'DELETE' 'FROM' ( ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) | ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) table_alias_name | ( ( 'ONLY' | ) table_name opt_index_flags ( '*' | ) ) 'AS' table_alias_name ) ( 'USING' ( ( table_ref ) ( ( ',' table_ref ) )* ) | ) ( ( 'WHERE' a_expr ) | ) ( sort_clause | ) ( limit_clause | ) ( 'RETURNING' target_list | 'RETURNING' 'NOTHING' | )
3 changes: 3 additions & 0 deletions pkg/ccl/backupccl/restore_schema_change_creation.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ func jobDescriptionFromMutationID(
}
}
jobDescBuilder.WriteString(")")
case *descpb.DescriptorMutation_MaterializedViewRefresh:
jobDescBuilder.WriteString("refreshing materialized view with primary key ")
jobDescBuilder.WriteString(t.MaterializedViewRefresh.NewPrimaryIndex.Name)
default:
return "", 0, errors.Newf("unsupported mutation %+v, while restoring table %+v", m, tableDesc)
}
Expand Down
42 changes: 42 additions & 0 deletions pkg/ccl/backupccl/testdata/backup-restore/materialized_view
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# We allow implicit access to non-admin users so that we can test
# with nodelocal.
new-cluster name=s1 allow-implicit-access
----

exec-sql
CREATE DATABASE testdb;
USE testdb;
CREATE TABLE testdb.t (a int primary key, b int);
CREATE MATERIALIZED VIEW testdb.mv AS SELECT a, b FROM testdb.t;
INSERT INTO testdb.t (a, b) VALUES (1, 2);
----

exec-sql
REFRESH MATERIALIZED VIEW mv;
----

exec-sql
INSERT INTO testdb.t (a, b) VALUES (2, 3);
----

exec-sql
REFRESH MATERIALIZED VIEW mv;
----

exec-sql
BACKUP INTO 'nodelocal://1/test/'
----


new-cluster name=s2 share-io-dir=s1 allow-implicit-access
----

exec-sql
RESTORE DATABASE testdb FROM LATEST IN 'nodelocal://1/test/' WITH new_db_name = 'newdb';
----

query-sql
SELECT * FROM newdb.mv;
----
1 2
2 3
2 changes: 1 addition & 1 deletion pkg/cmd/docgen/diagrams.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ var specs = []stmtSpec{
},
{
name: "delete_stmt",
inline: []string{"opt_with_clause", "with_clause", "cte_list", "table_expr_opt_alias_idx", "table_name_opt_idx", "opt_where_clause", "where_clause", "returning_clause", "opt_sort_clause", "opt_limit_clause", "opt_only", "opt_descendant"},
inline: []string{"opt_with_clause", "with_clause", "cte_list", "table_expr_opt_alias_idx", "table_name_opt_idx", "opt_where_clause", "where_clause", "returning_clause", "opt_sort_clause", "opt_limit_clause", "opt_only", "opt_descendant", "opt_using_clause", "from_list"},
replace: map[string]string{
"relation_expr": "table_name",
},
Expand Down
54 changes: 35 additions & 19 deletions pkg/cmd/roachtest/tests/npgsql_blocklist.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ var npgsqlBlocklist = blocklist{
`Npgsql.Tests.CopyTests(Multiplexing).Binary_roundtrip(True)`: "97181",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_binary_export_when_not_consumed_and_then_Dispose`: "97180",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_raw_binary_export_when_not_consumed_and_then_Dispose`: "97180",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_raw_binary_import`: "unknown",
`Npgsql.Tests.CopyTests(Multiplexing).Close_during_copy_throws`: "97181",
`Npgsql.Tests.CopyTests(Multiplexing).Dispose_in_middle_of_raw_binary_export`: "97181",
`Npgsql.Tests.CopyTests(Multiplexing).Dispose_in_middle_of_raw_binary_import`: "unknown",
Expand Down Expand Up @@ -409,13 +408,15 @@ var npgsqlBlocklist = blocklist{
`Npgsql.Tests.SchemaTests(Async).DataTypes`: "51480",
`Npgsql.Tests.SchemaTests(Async).Enum_in_non_public_schema`: "unknown",
`Npgsql.Tests.SchemaTests(Async).Enum_in_public_schema`: "51480",
`Npgsql.Tests.SchemaTests(Async).MetaDataCollections`: "bug in test",
`Npgsql.Tests.SchemaTests(Async).Precision_and_scale`: "unknown",
`Npgsql.Tests.SchemaTests(Async).Unique_constraint`: "unknown",
`Npgsql.Tests.SchemaTests(Async).Unique_index_composite`: "unknown",
`Npgsql.Tests.SchemaTests(Sync).Column_schema_data_types`: "41578",
`Npgsql.Tests.SchemaTests(Sync).DataTypes`: "51480",
`Npgsql.Tests.SchemaTests(Sync).Enum_in_non_public_schema`: "unknown",
`Npgsql.Tests.SchemaTests(Sync).Enum_in_public_schema`: "51480",
`Npgsql.Tests.SchemaTests(Sync).MetaDataCollections`: "bug in test",
`Npgsql.Tests.SchemaTests(Sync).Precision_and_scale`: "unknown",
`Npgsql.Tests.SchemaTests(Sync).Unique_constraint`: "unknown",
`Npgsql.Tests.SchemaTests(Sync).Unique_index_composite`: "unknown",
Expand Down Expand Up @@ -750,22 +751,37 @@ var npgsqlBlocklist = blocklist{
}

var npgsqlIgnoreList = blocklist{
`Npgsql.Tests.ConnectionTests(NonMultiplexing).ManyOpenClose`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).PhysicalConnectionInitializer_sync`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_binary_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_text_export_when_not_consumed_and_then_Dispose`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_text_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_bytea`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_commit_in_middle_of_row`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_direct_buffer`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_exception_does_not_commit`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_string_array`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Prepended_messages`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Write_null_values`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Wrong_table_definition_binary_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Wrong_table_definition_raw_binary_copy`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Write_null_values`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Wrong_format_binary_export`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Import_numeric`: "flaky",
`Npgsql.Tests.TransactionTests(NonMultiplexing).CommitAsync(Prepared)`: "flaky",
`Npgsql.Tests.CommandTests(NonMultiplexing).Statement_mapped_output_parameters(SequentialAccess)`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).PostgreSqlVersion_ServerVersion`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).Connector_not_initialized_exception`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).Many_open_close_with_transaction`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).ManyOpenClose`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).PhysicalConnectionInitializer_sync`: "flaky",
`Npgsql.Tests.ConnectionTests(NonMultiplexing).Timezone_connection_param`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_binary_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_raw_binary_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_text_export_when_not_consumed_and_then_Dispose`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Cancel_text_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_bytea`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_commit_in_middle_of_row`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_direct_buffer`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_exception_does_not_commit`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_numeric`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_string_array`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Import_string_with_buffer_length`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Prepended_messages`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Text_import(False)`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Text_import_empty`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Write_null_values`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Wrong_table_definition_binary_export`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Wrong_table_definition_binary_import`: "flaky",
`Npgsql.Tests.CopyTests(Multiplexing).Wrong_table_definition_raw_binary_copy`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Cancel_text_export_when_not_consumed_and_then_Dispose`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Import_numeric`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Import_string_with_buffer_length`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Write_column_out_of_bounds_throws`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Write_null_values`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Wrong_format_binary_export`: "flaky",
`Npgsql.Tests.CopyTests(NonMultiplexing).Wrong_format_binary_import`: "flaky",
`Npgsql.Tests.TransactionTests(NonMultiplexing).CommitAsync(Prepared)`: "flaky",
}
5 changes: 2 additions & 3 deletions pkg/cmd/roachtest/tests/rust_postgres_blocklist.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
package tests

var rustPostgresBlocklist = blocklist{
"binary_copy.read_basic": "No COPY TO support - https://github.com/cockroachdb/cockroach/issues/85571",
"binary_copy.read_basic": "No binary COPY TO support - https://github.com/cockroachdb/cockroach/issues/97180",
"binary_copy.read_big_rows": "default int size (int4 vs int8) mismatch",
"binary_copy.read_many_rows": "No COPY TO support - https://github.com/cockroachdb/cockroach/issues/85571",
"binary_copy.read_many_rows": "No binary COPY TO support - https://github.com/cockroachdb/cockroach/issues/97180",
"binary_copy.write_basic": "COPY FROM not supported in extended protocol",
"binary_copy.write_big_rows": "COPY FROM not supported in extended protocol",
"binary_copy.write_many_rows": "COPY FROM not supported in extended protocol",
Expand Down Expand Up @@ -42,7 +42,6 @@ var rustPostgresBlocklist = blocklist{
"test.binary_copy_out": "No COPY TO support - https://github.com/cockroachdb/cockroach/issues/85571",
"test.copy_in": "COPY FROM not supported in extended protocol",
"test.copy_in_abort": "COPY FROM not supported in extended protocol",
"test.copy_out": "No COPY TO support - https://github.com/cockroachdb/cockroach/issues/85571",
"test.nested_transactions": "default int size (int4 vs int8) mismatch",
"test.notice_callback": "unsupported feature - https://github.com/cockroachdb/cockroach/issues/17511",
"test.notifications_blocking_iter": "unsupported feature - https://github.com/cockroachdb/cockroach/issues/41522",
Expand Down
92 changes: 20 additions & 72 deletions pkg/kv/kvserver/client_raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"math/rand"
"reflect"
"strconv"
"strings"
"sync"
"sync/atomic"
"testing"
Expand All @@ -31,6 +32,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
Expand Down Expand Up @@ -1278,14 +1280,24 @@ func TestRequestsOnLaggingReplica(t *testing.T) {
log.Infof(ctx, "test: sending request")
timeoutCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
getRequest := getArgs(key)
_, pErr := kv.SendWrapped(timeoutCtx, partitionedStoreSender, getRequest)
require.NotNil(t, pErr, "unexpected success")
nlhe := pErr.GetDetail().(*kvpb.NotLeaseHolderError)
require.NotNil(t, nlhe, "expected NotLeaseholderError, got: %s", pErr)
require.False(t, nlhe.Lease.Empty())
require.NotNil(t, nlhe.Lease.Replica, "expected NotLeaseholderError with a known leaseholder, got: %s", pErr)
require.Equal(t, leaderReplicaID, nlhe.Lease.Replica.ReplicaID)
for {
getRequest := getArgs(key)
_, pErr := kv.SendWrapped(timeoutCtx, partitionedStoreSender, getRequest)
require.Error(t, pErr.GoError(), "unexpected success")
nlhe := &kvpb.NotLeaseHolderError{}
require.ErrorAs(t, pErr.GetDetail(), &nlhe)
// Someone else (e.g. the Raft scheduler) may have attempted to acquire the
// lease in the meanwhile, bumping the node's epoch and causing an
// ErrEpochIncremented, so we ignore these and try again.
if strings.Contains(nlhe.Error(), liveness.ErrEpochIncremented.Error()) { // no cause chain
t.Logf("got %s, retrying", nlhe)
continue
}
require.False(t, nlhe.Lease.Empty(), "expected NotLeaseholderError with a lease, got: %s", pErr)
require.NotNil(t, nlhe.Lease.Replica, "expected NotLeaseholderError with a known leaseholder, got: %s", pErr)
require.Equal(t, leaderReplicaID, nlhe.Lease.Replica.ReplicaID)
break
}
}

// TestRequestsOnFollowerWithNonLiveLeaseholder tests the availability of a
Expand Down Expand Up @@ -3822,70 +3834,6 @@ func TestReplicaTooOldGC(t *testing.T) {
})
}

func TestReplicaLazyLoad(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

stickyEngineRegistry := server.NewStickyInMemEnginesRegistry()
defer stickyEngineRegistry.CloseAllStickyInMemEngines()

stickyServerArgs := base.TestServerArgs{
StoreSpecs: []base.StoreSpec{
{
InMemory: true,
StickyInMemoryEngineID: "1",
},
},
RaftConfig: base.RaftConfig{
RaftTickInterval: 50 * time.Millisecond, // safe because there is only a single node
},
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEngineRegistry,
},
Store: &kvserver.StoreTestingKnobs{
DisableScanner: true,
},
},
}

ctx := context.Background()
tc := testcluster.StartTestCluster(t, 1,
base.TestClusterArgs{
ReplicationMode: base.ReplicationManual,
ServerArgs: stickyServerArgs,
})
defer tc.Stopper().Stop(ctx)

// Split so we can rely on RHS range being quiescent after a restart.
// We use UserTableDataMin to avoid having the range activated to
// gossip system table data.
splitKey := bootstrap.TestingUserTableDataMin()
tc.SplitRangeOrFatal(t, splitKey)

tc.StopServer(0)
tc.AddAndStartServer(t, stickyServerArgs)

// Wait for a bunch of raft ticks.
ticks := tc.GetFirstStoreFromServer(t, 1).Metrics().RaftTicks.Count
for targetTicks := ticks() + 3; ticks() < targetTicks; {
time.Sleep(time.Millisecond)
}

splitKeyAddr, err := keys.Addr(splitKey)
if err != nil {
t.Fatal(err)
}

replica := tc.GetFirstStoreFromServer(t, 1).LookupReplica(splitKeyAddr)
if replica == nil {
t.Fatalf("lookup replica at key %q returned nil", splitKey)
}
if replica.RaftStatus() != nil {
t.Fatalf("expected replica Raft group to be uninitialized")
}
}

func TestReplicateReAddAfterDown(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down
Loading

0 comments on commit 117dc40

Please sign in to comment.