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

opt: Allow matching constraints for leading locality tiers to be omitted #36644

Closed
andy-kimball opened this issue Apr 9, 2019 · 0 comments · Fixed by #36689
Closed

opt: Allow matching constraints for leading locality tiers to be omitted #36644

andy-kimball opened this issue Apr 9, 2019 · 0 comments · Fixed by #36689
Assignees

Comments

@andy-kimball
Copy link
Contributor

REPRO:

  1. Start 3 nodes.
./cockroach start --insecure --locality=cloud=gce,region=us-east1,zone=us-east1-b --store=node1 --listen-addr=localhost:26257 --http-addr=localhost:8080
./cockroach start --insecure --locality=cloud=gce,region=us-west1,zone=us-west1-b --store=node2 --listen-addr=localhost:26258 --http-addr=localhost:8081 --join=localhost:26257
./cockroach start --insecure --locality=cloud=gce,region=europe-west2,zone=europe-west2-b --store=node3 --listen-addr=localhost:26259 --http-addr=localhost:8082 --join=localhost:26257
  1. Connect to node tidy up some correctness issues reported by go vet #1 via the SQL CLI and issue the following commands:
CREATE TABLE t (
    id INT PRIMARY KEY,
    UNIQUE INDEX idx (id ASC)
);

ALTER TABLE t CONFIGURE ZONE USING constraints = '[+cloud=gce,+region=us-west1,+zone=us-west1-b]';
ALTER INDEX t@idx CONFIGURE ZONE USING constraints = '[+zone=us-east1-b]';

EXPLAIN (OPT) SELECT id FROM t WHERE id = 10;

EXPECTED: The query should use the t@idx index, since it matches the zone, even though it does not explicitly match the cloud or region. Allowing this behavior makes it more convenient to use the zone locality feature.

ACTUAL: The query uses the primary index.

@andy-kimball andy-kimball self-assigned this Apr 9, 2019
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Apr 9, 2019
This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Apr 10, 2019
This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Apr 10, 2019
This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Apr 10, 2019
This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Apr 10, 2019
This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes cockroachdb#36642
Fixes cockroachdb#36644

Release note: None
craig bot pushed a commit that referenced this issue Apr 10, 2019
34258: storage: backpressure SSTs before other writes if L0 fills up r=dt a=dt

Currently we start compacting as soon as there are two or more files in
L0. If we reach 20, RocksDB will start slowing all writes in an attempt
to help the compactor catch up.

This global slowdown will affect foreground traffic, liveness, etc and
could significantly impact latency of live traffic or even availability.

Directly ingesting large numbers of small files could make it easier to
hit this limit than it usually is with normal writes -- normal writes
are buffered in the memtable and flushed all at once but direct
ingestion could add potentially lots of files rather quickly.

Additionally, SST ingetions are currently only done by bulk operations
like RESTORE, IMPORT or index backfills. These are all less latency
sensitive then foreground traffic, so we'd prefer to backpressure them
before we risk approaching the global slowdown trigger.

Release note (performance improvement): backpressure bulk operations before other traffic.

36689: opt: Fix and improve zone-based index selection r=andy-kimball a=andy-kimball

This PR fixes a couple of related issues with zone-based index selection.

First, placeholder zones were incorrectly discarded. Placeholder zones are
used when there are indexes with Constraints/Leaseholder Preferences on a
table that doesn't have either of those things. The fix is to merge the
indexes placeholder into the table zone.

Second, the optimizer required that a prefix of the locality tiers match
constraints. That is no longer required after this commit. For example, if
locality=region=us;dc=east, then it now matches any of these constraint
sets equally well:

  [+locality=region=us;+dc=east]
  [+dc=east]
  [+dc=east,+dc=west]

A missing constraint match is OK (i.e. region=us), as long as a more specific
locality tier matches (i.e. dc=east).

Fixes #36642
Fixes #36644

Release note: None

Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Andrew Kimball <[email protected]>
@craig craig bot closed this as completed in #36689 Apr 10, 2019
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 a pull request may close this issue.

1 participant