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

asim: replica load adapter #81004

Merged
merged 1 commit into from
May 17, 2022
Merged

Conversation

kvoli
Copy link
Collaborator

@kvoli kvoli commented May 4, 2022

This patch introduces an adapter for the simulator state of load on
stores and replicas, into the datastructures supported by the allocator.

Replica load may be experimented with in the future and this patch
defines an interface for interacting with recording and summarizing
replica load.

Store load is partially an aggregaton of replica load and
therefore relies on the replica load interface to calculate capacity.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@kvoli kvoli force-pushed the 220426.asim-load-adapter branch from facb285 to 47d5752 Compare May 4, 2022 22:29
@kvoli kvoli marked this pull request as ready for review May 4, 2022 22:29
@kvoli kvoli requested a review from a team as a code owner May 4, 2022 22:29
@kvoli kvoli requested a review from lidorcarmel May 4, 2022 22:29
@kvoli kvoli self-assigned this May 5, 2022
Copy link
Contributor

@lidorcarmel lidorcarmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, minor nits.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @kvoli)


-- commits line 11 at r1:
typo?

Code quote:

aggregaton

pkg/kv/kvserver/asim/load.go line 21 at r1 (raw file):

// order to record and report load events.
type ReplicaLoad interface {
	// AppluLoad applies a load event to a replica.

typo

Code quote:

AppluLoad

pkg/kv/kvserver/asim/load.go line 65 at r1 (raw file):

// replica within the store.
func (s *Store) Capacity() roachpb.StoreCapacity {
	capacity := roachpb.StoreCapacity{}

just to make sure I understand correctly - we will need to populate more fields for capacity here, right? such as roachpb.StoreCapacity.capacity and available etc.
worth adding a todo?

This patch introduces an adapter for the simulator state of load on
stores and replicas, into the datastructures supported by the allocator.

Replica load may be experimented with in the future and this patch
defines an interface for interacting with recording and summarizing
replica load.

Store load is partially an aggregation of replica load and
therefore relies on the replica load interface to calculate capacity.

Release note: None
@kvoli kvoli force-pushed the 220426.asim-load-adapter branch from 47d5752 to 6152275 Compare May 17, 2022 14:28
Copy link
Collaborator Author

@kvoli kvoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TYFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lidorcarmel)


-- commits line 11 at r1:

Previously, lidorcarmel (Lidor Carmel) wrote…

typo?

Updated.


pkg/kv/kvserver/asim/load.go line 21 at r1 (raw file):

Previously, lidorcarmel (Lidor Carmel) wrote…

typo

Updated.


pkg/kv/kvserver/asim/load.go line 65 at r1 (raw file):

Previously, lidorcarmel (Lidor Carmel) wrote…

just to make sure I understand correctly - we will need to populate more fields for capacity here, right? such as roachpb.StoreCapacity.capacity and available etc.
worth adding a todo?

Added.

@kvoli
Copy link
Collaborator Author

kvoli commented May 17, 2022

bors r+

@craig
Copy link
Contributor

craig bot commented May 17, 2022

Build succeeded:

@craig craig bot merged commit 05396b3 into cockroachdb:master May 17, 2022
@kvoli kvoli mentioned this pull request Jun 8, 2022
16 tasks
@kvoli kvoli added the A-kv-simulation Relating to allocation simulation. label Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-simulation Relating to allocation simulation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants