Skip to content

Commit

Permalink
Merge #102629 #107983 #111128
Browse files Browse the repository at this point in the history 
102629: kvserver: stop using deprecated snapshot fields r=kvoli a=andrewbaptist

A number of fields in snapshot messages are no longer relevant. This PR stops using those fields completely so that in the next release they can be removed from the protocol completely.

Release note: None
Epic: none

107983: asim: add zone ctor to improve default StoresPerNode handling r=kvoli a=wenyihu6

Previously, simulated availability zones were initialized with struct literals.
If `StoresPerNode` was left uninitialized, a default value of 1 is set before its
use in functions. This was less ideal and seemed more error-prone. To improve
this, this patch adds two constructors for simulated availability zones: one
with a default of one store per node and one with custom stores per node.

Release Note: None

Epic: None

111128: ui: fix statement diagnostics page on max size error r=j82w a=j82w

Problem:
The SQL API has a maximum size of 50KB. The statement diagnostics page can easily hit this limit with large queries and/or large number of statement diagnostics. The page would fail to load and show no results.

Fix:
Add a new method to the SQL API to keep pulling results until no more max size errors are returned.

Fixes: #108403

https://www.loom.com/share/8c88e44e03044eda8df06e23a3bc501a?sid=adc82737-1568-483d-bf48-6509779713e5

Release note (ui change): Fixes the statement diagnostics page when the response hit a max size error.

Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: wenyihu6 <[email protected]>
Co-authored-by: j82w <[email protected]>
  • Loading branch information
@andrewbaptist @wenyihu6
4 people committed Sep 26, 2023
4 parents 0453b28 + beb4d8b + bd2385a + 236b581 commit 5d61611
Show file tree
Hide file tree
Showing 15 changed files with 254 additions and 229 deletions.
67 changes: 43 additions & 24 deletions pkg/kv/kvserver/asim/state/config_loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ var SingleRegionConfig = ClusterInfo{
{
Name: "US",
Zones: []Zone{
{Name: "US_1", NodeCount: 5},
{Name: "US_2", NodeCount: 5},
{Name: "US_3", NodeCount: 5},
NewZoneWithSingleStore("US_1", 5),
NewZoneWithSingleStore("US_2", 5),
NewZoneWithSingleStore("US_3", 5),
},
},
},
Expand All @@ -52,9 +52,9 @@ var SingleRegionMultiStoreConfig = ClusterInfo{
{
Name: "US",
Zones: []Zone{
{Name: "US_1", NodeCount: 1, StoresPerNode: 5},
{Name: "US_2", NodeCount: 1, StoresPerNode: 5},
{Name: "US_3", NodeCount: 1, StoresPerNode: 5},
NewZone("US_1", 1, 5),
NewZone("US_2", 1, 5),
NewZone("US_3", 1, 5),
},
},
},
Expand All @@ -67,25 +67,25 @@ var MultiRegionConfig = ClusterInfo{
{
Name: "US_East",
Zones: []Zone{
{Name: "US_East_1", NodeCount: 4},
{Name: "US_East_2", NodeCount: 4},
{Name: "US_East_3", NodeCount: 4},
NewZoneWithSingleStore("US_East_1", 4),
NewZoneWithSingleStore("US_East_2", 4),
NewZoneWithSingleStore("US_East_3", 4),
},
},
{
Name: "US_West",
Zones: []Zone{
{Name: "US_West_1", NodeCount: 4},
{Name: "US_West_2", NodeCount: 4},
{Name: "US_West_3", NodeCount: 4},
NewZoneWithSingleStore("US_West_1", 4),
NewZoneWithSingleStore("US_West_2", 4),
NewZoneWithSingleStore("US_West_3", 4),
},
},
{
Name: "EU",
Zones: []Zone{
{Name: "EU_1", NodeCount: 4},
{Name: "EU_2", NodeCount: 4},
{Name: "EU_3", NodeCount: 4},
NewZoneWithSingleStore("EU_1", 4),
NewZoneWithSingleStore("EU_2", 4),
NewZoneWithSingleStore("EU_3", 4),
},
},
},
Expand All @@ -98,24 +98,24 @@ var ComplexConfig = ClusterInfo{
{
Name: "US_East",
Zones: []Zone{
{Name: "US_East_1", NodeCount: 1},
{Name: "US_East_2", NodeCount: 2},
{Name: "US_East_3", NodeCount: 3},
{Name: "US_East_3", NodeCount: 10},
NewZoneWithSingleStore("US_East_1", 1),
NewZoneWithSingleStore("US_East_2", 2),
NewZoneWithSingleStore("US_East_3", 3),
NewZoneWithSingleStore("US_East_3", 10),
},
},
{
Name: "US_West",
Zones: []Zone{
{Name: "US_West_1", NodeCount: 2},
NewZoneWithSingleStore("US_West_1", 2),
},
},
{
Name: "EU",
Zones: []Zone{
{Name: "EU_1", NodeCount: 3},
{Name: "EU_2", NodeCount: 3},
{Name: "EU_3", NodeCount: 4},
NewZoneWithSingleStore("EU_1", 3),
NewZoneWithSingleStore("EU_2", 3),
NewZoneWithSingleStore("EU_3", 4),
},
},
},
Expand Down Expand Up @@ -253,6 +253,25 @@ type Zone struct {
StoresPerNode int
}

// NewZoneWithSingleStore is a constructor for a simulated availability zone,
// taking zone name, node count, and a default of one store per node.
func NewZoneWithSingleStore(name string, nodeCount int) Zone {
return NewZone(name, nodeCount, 1)
}

// NewZone is a constructor for a simulated availability zone, taking zone name,
// node count, and custom stores per node.
func NewZone(name string, nodeCount int, storesPerNode int) Zone {
if storesPerNode < 1 {
panic(fmt.Sprintf("storesPerNode cannot be less than one but found %v", storesPerNode))
}
return Zone{
Name: name,
NodeCount: nodeCount,
StoresPerNode: storesPerNode,
}
}

// Region is a simulated region which contains one or more zones.
type Region struct {
Name string
Expand Down Expand Up @@ -328,7 +347,7 @@ func LoadClusterInfo(c ClusterInfo, settings *config.SimulationSettings) State {
s.SetNodeLocality(node.NodeID(), locality)
storesRequired := z.StoresPerNode
if storesRequired < 1 {
storesRequired = 1
panic(fmt.Sprintf("storesPerNode cannot be less than one but found %v", storesRequired))
}
for store := 0; store < storesRequired; store++ {
if newStore, ok := s.AddStore(node.NodeID()); !ok {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/asim/state/new_state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func ClusterInfoWithDistribution(
availableNodes -= allocatedNodes
ret.Regions[i] = Region{
Name: name,
Zones: []Zone{{Name: name + "_1", NodeCount: allocatedNodes, StoresPerNode: storesPerNode}},
Zones: []Zone{NewZone(name+"_1", allocatedNodes, storesPerNode)},
}
}

Expand Down
28 changes: 14 additions & 14 deletions pkg/kv/kvserver/asim/tests/testdata/rand/rand_cluster
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ sample1: pass
sample2: start running
configurations generated using seed 1926012586526624009
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -60,9 +60,9 @@ generating settings configurations using static option
sample1: start running
configurations generated using seed 608747136543856411
loaded cluster with
region:US_East [zone=US_East_1(nodes=1,stores=0), zone=US_East_2(nodes=2,stores=0), zone=US_East_3(nodes=3,stores=0), zone=US_East_3(nodes=10,stores=0)]
region:US_West [zone=US_West_1(nodes=2,stores=0)]
region:EU [zone=EU_1(nodes=3,stores=0), zone=EU_2(nodes=3,stores=0), zone=EU_3(nodes=4,stores=0)]
region:US_East [zone=US_East_1(nodes=1,stores=1), zone=US_East_2(nodes=2,stores=1), zone=US_East_3(nodes=3,stores=1), zone=US_East_3(nodes=10,stores=1)]
region:US_West [zone=US_West_1(nodes=2,stores=1)]
region:EU [zone=EU_1(nodes=3,stores=1), zone=EU_2(nodes=3,stores=1), zone=EU_3(nodes=4,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -92,9 +92,9 @@ sample1: pass
sample2: start running
configurations generated using seed 1926012586526624009
loaded cluster with
region:US_East [zone=US_East_1(nodes=4,stores=0), zone=US_East_2(nodes=4,stores=0), zone=US_East_3(nodes=4,stores=0)]
region:US_West [zone=US_West_1(nodes=4,stores=0), zone=US_West_2(nodes=4,stores=0), zone=US_West_3(nodes=4,stores=0)]
region:EU [zone=EU_1(nodes=4,stores=0), zone=EU_2(nodes=4,stores=0), zone=EU_3(nodes=4,stores=0)]
region:US_East [zone=US_East_1(nodes=4,stores=1), zone=US_East_2(nodes=4,stores=1), zone=US_East_3(nodes=4,stores=1)]
region:US_West [zone=US_West_1(nodes=4,stores=1), zone=US_West_2(nodes=4,stores=1), zone=US_West_3(nodes=4,stores=1)]
region:EU [zone=EU_1(nodes=4,stores=1), zone=EU_2(nodes=4,stores=1), zone=EU_3(nodes=4,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -128,9 +128,9 @@ sample2: pass
sample3: start running
configurations generated using seed 3534334367214237261
loaded cluster with
region:US_East [zone=US_East_1(nodes=1,stores=0), zone=US_East_2(nodes=2,stores=0), zone=US_East_3(nodes=3,stores=0), zone=US_East_3(nodes=10,stores=0)]
region:US_West [zone=US_West_1(nodes=2,stores=0)]
region:EU [zone=EU_1(nodes=3,stores=0), zone=EU_2(nodes=3,stores=0), zone=EU_3(nodes=4,stores=0)]
region:US_East [zone=US_East_1(nodes=1,stores=1), zone=US_East_2(nodes=2,stores=1), zone=US_East_3(nodes=3,stores=1), zone=US_East_3(nodes=10,stores=1)]
region:US_West [zone=US_West_1(nodes=2,stores=1)]
region:EU [zone=EU_1(nodes=3,stores=1), zone=EU_2(nodes=3,stores=1), zone=EU_3(nodes=4,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -179,7 +179,7 @@ sample1: pass
sample2: start running
configurations generated using seed 1926012586526624009
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand All @@ -188,9 +188,9 @@ sample2: pass
sample3: start running
configurations generated using seed 3534334367214237261
loaded cluster with
region:US_East [zone=US_East_1(nodes=1,stores=0), zone=US_East_2(nodes=2,stores=0), zone=US_East_3(nodes=3,stores=0), zone=US_East_3(nodes=10,stores=0)]
region:US_West [zone=US_West_1(nodes=2,stores=0)]
region:EU [zone=EU_1(nodes=3,stores=0), zone=EU_2(nodes=3,stores=0), zone=EU_3(nodes=4,stores=0)]
region:US_East [zone=US_East_1(nodes=1,stores=1), zone=US_East_2(nodes=2,stores=1), zone=US_East_3(nodes=3,stores=1), zone=US_East_3(nodes=10,stores=1)]
region:US_West [zone=US_West_1(nodes=2,stores=1)]
region:EU [zone=EU_1(nodes=3,stores=1), zone=EU_2(nodes=3,stores=1), zone=EU_3(nodes=4,stores=1)]
basic ranges with placement_type=even, ranges=10, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/asim/tests/testdata/rand/rand_event
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ generating settings configurations using static option
sample1: start running
configurations generated using seed 7894140303635748408
loaded cluster with
region:US_East [zone=US_East_1(nodes=1,stores=0), zone=US_East_2(nodes=2,stores=0), zone=US_East_3(nodes=3,stores=0), zone=US_East_3(nodes=10,stores=0)]
region:US_West [zone=US_West_1(nodes=2,stores=0)]
region:EU [zone=EU_1(nodes=3,stores=0), zone=EU_2(nodes=3,stores=0), zone=EU_3(nodes=4,stores=0)]
region:US_East [zone=US_East_1(nodes=1,stores=1), zone=US_East_2(nodes=2,stores=1), zone=US_East_3(nodes=3,stores=1), zone=US_East_3(nodes=10,stores=1)]
region:US_West [zone=US_West_1(nodes=2,stores=1)]
region:EU [zone=EU_1(nodes=3,stores=1), zone=EU_2(nodes=3,stores=1), zone=EU_3(nodes=4,stores=1)]
basic ranges with placement_type=even, ranges=1, key_space=200000, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=12, number of assertion events=12
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/asim/tests/testdata/rand/rand_ranges
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ sample1: pass
sample2: start running
configurations generated using seed 2643318057788968173
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
randomized ranges with placement_type=random, ranges=944, key_space=1357, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand All @@ -58,7 +58,7 @@ sample2: pass
sample3: start running
configurations generated using seed 6972490225919430754
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
randomized ranges with placement_type=random, ranges=479, key_space=1003, replication_factor=3, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -173,7 +173,7 @@ sample1: pass
sample2: start running
configurations generated using seed 2643318057788968173
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
randomized ranges with placement_type=random, ranges=944, key_space=150098, replication_factor=1, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down Expand Up @@ -205,7 +205,7 @@ over replicated:
sample3: start running
configurations generated using seed 6972490225919430754
loaded cluster with
region:US [zone=US_1(nodes=5,stores=0), zone=US_2(nodes=5,stores=0), zone=US_3(nodes=5,stores=0)]
region:US [zone=US_1(nodes=5,stores=1), zone=US_2(nodes=5,stores=1), zone=US_3(nodes=5,stores=1)]
randomized ranges with placement_type=random, ranges=479, key_space=199954, replication_factor=1, bytes=0
basic load with rw_ratio=0.00, rate=0.00, skewed_access=false, min_block_size=1, max_block_size=1, min_key=1, max_key=200000
number of mutation events=0, number of assertion events=0
Expand Down
5 changes: 1 addition & 4 deletions pkg/kv/kvserver/client_merge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ func mergeCheckingTimestampCaches(
var snapshotFilter func(kvserver.IncomingSnapshot)
beforeSnapshotSSTIngestion := func(
inSnap kvserver.IncomingSnapshot,
snapType kvserverpb.SnapshotRequest_Type,
_ []string,
) error {
filterMu.Lock()
Expand Down Expand Up @@ -3726,7 +3725,6 @@ func TestStoreRangeMergeRaftSnapshot(t *testing.T) {
rangeIds := make(map[string]roachpb.RangeID, 4)
beforeSnapshotSSTIngestion := func(
inSnap kvserver.IncomingSnapshot,
snapType kvserverpb.SnapshotRequest_Type,
sstNames []string,
) error {
// Only verify snapshots of type VIA_SNAPSHOT_QUEUE and on the range under
Expand All @@ -3735,8 +3733,7 @@ func TestStoreRangeMergeRaftSnapshot(t *testing.T) {
// there are too many keys and the other replicated keys are verified later
// on in the test. This function verifies that the subsumed replicas have
// been handled properly.
if snapType != kvserverpb.SnapshotRequest_VIA_SNAPSHOT_QUEUE ||
inSnap.Desc.RangeID != rangeIds[string(keyA)] {
if inSnap.Desc.RangeID != rangeIds[string(keyA)] {
return nil
}

Expand Down
2 changes: 0 additions & 2 deletions pkg/kv/kvserver/client_replica_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4736,7 +4736,6 @@ func TestTenantID(t *testing.T) {
Store: &kvserver.StoreTestingKnobs{
BeforeSnapshotSSTIngestion: func(
snapshot kvserver.IncomingSnapshot,
requestType kvserverpb.SnapshotRequest_Type,
strings []string,
) error {
if snapshot.Desc.RangeID == repl.RangeID {
Expand Down Expand Up @@ -4815,7 +4814,6 @@ func TestUninitializedMetric(t *testing.T) {
Store: &kvserver.StoreTestingKnobs{
BeforeSnapshotSSTIngestion: func(
snapshot kvserver.IncomingSnapshot,
_ kvserverpb.SnapshotRequest_Type,
_ []string,
) error {
if snapshot.Desc.RangeID == repl.RangeID {
Expand Down
19 changes: 10 additions & 9 deletions pkg/kv/kvserver/kvserverpb/raft.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,16 +214,17 @@ message SnapshotRequest {

// The priority of the snapshot.
// Deprecated, prefer sender_queue_priority.
// TODO(abaptist): Remove this field for v23.1.
Priority priority = 6;
// TODO(abaptist): Remove this field when v23.1 compatibility is dropped.
Priority deprecated_priority = 6;

// The strategy of the snapshot.
Strategy strategy = 7;
// TODO(abaptist): Remove this field when v23.1 compatibility is dropped.
Strategy deprecated_strategy = 7;

// The type of the snapshot.
// Deprecated, prefer sender_queue_name.
// TODO(abaptist): Remove this field for v23.1.
Type type = 9;
// TODO(abaptist): Remove this field when v23.1 compatibility is dropped.
Type deprecated_type = 9;

// Whether the snapshot uses the unreplicated RaftTruncatedState or not.
// This is always true for snapshots generated in v21.1+ clusters. In v20.2
Expand Down Expand Up @@ -378,12 +379,12 @@ message DelegateSendSnapshotRequest {
roachpb.ReplicaDescriptor delegated_sender = 4 [(gogoproto.nullable) = false];

// The priority of the snapshot.
// TODO(abaptist): Remove this field for v23.1.
SnapshotRequest.Priority priority = 5;
// TODO(abaptist): Remove this field when v23.1 compatibility is dropped.
SnapshotRequest.Priority deprecated_priority = 5;

// The type of the snapshot.
// TODO(abaptist): Remove this field for v23.1.
SnapshotRequest.Type type = 6;
// TODO(abaptist): Remove this field when v23.1 compatibility is dropped.
SnapshotRequest.Type deprecated_type = 6;

// The Raft term of the coordinator (in most cases the leaseholder) replica.
// The term is used during snapshot receiving to reject messages from an older term.
Expand Down
Loading

0 comments on commit 5d61611

Please sign in to comment.