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

allocator: exclude lease transfer targets with a send queue #128028

Closed
Tracked by #123509
kvoli opened this issue Jul 31, 2024 · 0 comments · Fixed by #131457
Closed
Tracked by #123509

allocator: exclude lease transfer targets with a send queue #128028

kvoli opened this issue Jul 31, 2024 · 0 comments · Fixed by #131457
Assignees
Labels
A-replication-admission-control-v2 Related to introduction of replication AC v2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@kvoli
Copy link
Collaborator

kvoli commented Jul 31, 2024

The allocator will transfer leases to replicas which are considered valid. A replica may be considered invalid if its store hasn't heart beated liveness recently enough, may require a snapshot or is otherwise unhealthy.

Extend the valid checks to also exclude replicas which currently have a send queue from transfer consideration.

See prototype commit adding this functionality 6b7b6f7

Jira issue: CRDB-40761

Epic CRDB-37515

@kvoli kvoli added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-replication-admission-control-v2 Related to introduction of replication AC v2 labels Jul 31, 2024
@blathers-crl blathers-crl bot added the T-kv KV Team label Jul 31, 2024
@github-project-automation github-project-automation bot moved this to Incoming in KV Aug 28, 2024
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 26, 2024
Add a new method to the `RangeController`,
`SendStreamStatsRaftMuLocked()`, which returns a map of
replica IDs -> replica send stream stats:

```
// ReplicaSendStreamStats contains the stats for a replica send stream that may
// be used to inform placement decisions pertaining to the replica.
type ReplicaSendStreamStats struct {
	// Closed is true iff the send stream is closed.
	Closed bool
	// SendQueueSizeBytes is the size of the send queue entries in bytes. This
	// roughly represents the amount of data that is buffered in the send queue,
	// waiting for available tokens before being sent. A large value here may
	// indicate that the replica's store is not able to keep up with the rate of
	// incoming replication.
	SendQueueSizeBytes int64
}
```

This method can be used to determine appropriate lease transfer targets.

Part of: cockroachdb#128028
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 26, 2024
Previously, it was possible to transfer a lease to a replica which had a
raft send queue, likely unable to keep up once acquiring the lease.

As a result, the leaseholder would throttle incoming replication traffic
before evaluation because it waited in the store work queue, or for
elastic work, waited for available flow tokens.

Using the new `RangeController` method exposing information about
replicas' send queue, `SendStreamStatsRaftMuLocked()`, exclude voter
candidates as lease transfer targets if:

```
send_queue_size > 0 bytes OR
replicaSendStream is closed
```

Resolves: cockroachdb#128028
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 30, 2024
Add a new method to the `RangeController`,
`SendStreamStatsRaftMuLocked()`, which returns a map of
replica IDs -> replica send stream stats:

```
// ReplicaSendStreamStats contains the stats for a replica send stream that may
// be used to inform placement decisions pertaining to the replica.
type ReplicaSendStreamStats struct {
	// IsStateReplicate is true iff the replica is being sent entries.
	IsStateReplicate bool
	// HasSendQueue is true when a replica has a non-zero amount of queued
	// entries waiting on flow tokens to be sent.
	//
	// This is only populated when IsStateReplicate is true.
	HasSendQueue bool
}
```

This method can be used to determine appropriate lease transfer targets.

Part of: cockroachdb#128028
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 30, 2024
Previously, it was possible to transfer a lease to a replica which had a
raft send queue, likely unable to keep up once acquiring the lease.

As a result, the leaseholder would throttle incoming replication traffic
before evaluation because it waited in the store work queue, or for
elastic work, waited for available flow tokens.

Using the new `RangeController` method exposing information about
replicas' send queue, `SendStreamStats()`, exclude voter candidates as
lease transfer targets if:

```
HasSendQueue is true OR
IsStateReplicate is false
```

Resolves: cockroachdb#128028
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Oct 1, 2024
Add a new method to the `RangeController`,
`SendStreamStats()`, which returns a map of replica IDs -> replica send
stream stats:

```
// ReplicaSendStreamStats contains the stats for a replica send stream that may
// be used to inform placement decisions pertaining to the replica.
type ReplicaSendStreamStats struct {
	// IsStateReplicate is true iff the replica is being sent entries.
	IsStateReplicate bool
	// HasSendQueue is true when a replica has a non-zero amount of queued
	// entries waiting on flow tokens to be sent.
	//
	// This is only populated when IsStateReplicate is true.
	HasSendQueue bool
}
```

This method can be used to determine appropriate lease transfer targets.

Part of: cockroachdb#128028
Release note: None
craig bot pushed a commit that referenced this issue Oct 1, 2024
131457: kvserver: exclude lease transfer targets with a send queue  r=sumeerbhola a=kvoli

First commit from #131504.


---

Add a new method to the `RangeController`,
`SendStreamStatsRaftMuLocked()`, which returns a map of
replica IDs -> replica send stream stats:

```
// ReplicaSendStreamStats contains the stats for a replica send stream that may
// be used to inform placement decisions pertaining to the replica.
type ReplicaSendStreamStats struct {
	// Closed is true iff the send stream is closed.
	Closed bool
	// SendQueueSizeBytes is the size of the send queue entries in bytes. This
	// roughly represents the amount of data that is buffered in the send queue,
	// waiting for available tokens before being sent. A large value here may
	// indicate that the replica's store is not able to keep up with the rate of
	// incoming replication.
	SendQueueSizeBytes int64
}
```

This method can be used to determine appropriate lease transfer targets.

---

Previously, it was possible to transfer a lease to a replica which had a
raft send queue, likely unable to keep up once acquiring the lease.

As a result, the leaseholder would throttle incoming replication traffic
before evaluation because it waited in the store work queue, or for
elastic work, waited for available flow tokens.

Using the new `RangeController` method exposing information about
replicas' send queue, `SendStreamStats()`, exclude voter candidates as
lease transfer targets if:

```
HasSendQueue is true OR
IsStateReplicate is false
```

Resolves: #128028
Release note: None

131613: sql: update reader tenant keywords r=msbutler a=azhu-crl

Rename `READ CAPABILITIES` to `READ VIRTUAL CLUSTER` as keywords to enable reader tenant.

Epic: CRDB-23575
Release note: None

Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: Anne Zhu <[email protected]>
@craig craig bot closed this as completed in 2d803cf Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-replication-admission-control-v2 Related to introduction of replication AC v2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
No open projects
Status: Incoming
Development

Successfully merging a pull request may close this issue.

1 participant