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: refactor StorePool usage to interface #91461

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

AlexTalks
Copy link
Contributor

@AlexTalks AlexTalks commented Nov 8, 2022

This change refactors the usage of StorePool in the allocator to a new interface, AllocatorStorePool, in order to be able to utilize a store pool with overriden liveness to properly evaluate decommission pre-flight checks.

Part of #91570.

Release note: None

@AlexTalks AlexTalks requested a review from kvoli November 8, 2022 04:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@AlexTalks AlexTalks marked this pull request as ready for review November 8, 2022 04:25
@AlexTalks AlexTalks requested a review from a team as a code owner November 8, 2022 04:25
@AlexTalks AlexTalks force-pushed the dpf_allocator_storepool branch 2 times, most recently from b25e909 to f8c8579 Compare November 9, 2022 04:56
Copy link
Collaborator

@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.

:lgtm:

We could be more aggressive in the trimming the iface but this is a strict improvement from my viewpoint.

Need to run dev gen and crlfmt.

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

@AlexTalks AlexTalks force-pushed the dpf_allocator_storepool branch from f8c8579 to dd6ff7c Compare November 15, 2022 20:21
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 16, 2022
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new `OverrideStorePool` struct
which can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets.  This `OverrideStorePool` is
backed by an existing actual `StorePool`, which retains the majority of
its logic.

Depends on cockroachdb#91461.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 16, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
@AlexTalks AlexTalks force-pushed the dpf_allocator_storepool branch 2 times, most recently from 8c013de to 9649ea1 Compare November 17, 2022 04:31
@AlexTalks
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 17, 2022

Build failed:

@AlexTalks
Copy link
Contributor Author

bors cancel

This change refactors the usage of `StorePool` in the allocator to a new
interface, `AllocatorStorePool`, in order to be able to utilize a store
pool with overriden liveness to properly evaluate decommission
pre-flight checks.

Part of cockroachdb#91570.

Release note: None
@AlexTalks AlexTalks force-pushed the dpf_allocator_storepool branch from 9649ea1 to a5daa80 Compare November 17, 2022 19:07
@AlexTalks
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 17, 2022

Build succeeded:

@craig craig bot merged commit 777bf2b into cockroachdb:master Nov 17, 2022
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 18, 2022
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new `OverrideStorePool` struct
which can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets.  This `OverrideStorePool` is
backed by an existing actual `StorePool`, which retains the majority of
its logic.

Depends on cockroachdb#91461.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 18, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 23, 2022
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new `OverrideStorePool` struct
which can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets.  This `OverrideStorePool` is
backed by an existing actual `StorePool`, which retains the majority of
its logic.

Depends on cockroachdb#91461.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Nov 23, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 9, 2022
While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new `OverrideStorePool` struct
which can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets.  This `OverrideStorePool` is
backed by an existing actual `StorePool`, which retains the majority of
its logic.

Depends on cockroachdb#91461.

Part of cockroachdb#91570.

Release note: None
craig bot pushed a commit that referenced this pull request Dec 9, 2022
90649: sql: support indexes in regclass cast and pg_table_is_visible r=ajwerner a=rafiss

fixes #88097

### sql: remove deprecated function from DatabaseCatalog

The ParseQualifiedTableName function was deprecated and unused.

### sql: support casts from index name to regclass

Release note (sql change): Casts from index name to regclass are now
supported. Previously, only table names could be cast to regclass.

### sql: add index on pg_namespace(oid)

This will allow us to efficiently join to this table.

### sql: fix pg_table_is_visible to handle indexes

This uses the internal executor now to do the introspection using
pg_class where everything can be looked up all at once.

There's an increase in number of round trips, but it's a constant
factor.

Release note (bug fix): Fixed the pg_table_is_visible builtin function
so it correctly reports visibility of indexes based on the current
search_path.

91965: allocator: add support for store pool liveness overrides r=AlexTalks a=AlexTalks

While previously the allocator only evaluated using liveness obtained
from gossip, this change introduces a new `OverrideStorePool` struct
which can be used to override the liveness of a node for the purposes of
evaluating allocator actions and targets.  This `OverrideStorePool` is
backed by an existing actual `StorePool`, which retains the majority of
its logic.

Depends on #91461.

Part of #91570.

Release note: None

93146: sql/sem: add check for interval for asof.DatumToHLC() r=rafiss a=ZhouXing19

Currently, if the given interval for `AS OF SYSTEM TIME interval` is a small postive duration, the query can incorrectly pass. It's because when we call `clock.Now()`, it has passed the threashold ('statement timestamp + duration').

Now we add a check for the duration value. It has to be negative, with the absolute value greater than a nanosecond.

fixes #91021
link epic CRDB-17785

Release note (bug fix): add restriction for duration value for AS OF SYSTEM TIME statement.

93340: roachtest: add flaky test to activerecord ignore list r=ZhouXing19 a=andyyang890

Fixes #93189

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Alex Sarkesian <[email protected]>
Co-authored-by: Jane Xing <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 15, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 19, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 20, 2022
This change adds methods to be to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 21, 2022
This change adds methods to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
AlexTalks added a commit to AlexTalks/cockroach that referenced this pull request Dec 21, 2022
This change adds methods to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
adityamaru pushed a commit to adityamaru/cockroach that referenced this pull request Dec 22, 2022
This change adds methods to evaluate allocator actions and targets
utilizing a passed-in `StorePool` object, allowing for the allocator to
consider potential scenarios rather than those simply based on the
current liveness.

Depends on cockroachdb#91461, cockroachdb#91965.

Part of cockroachdb#91570.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants