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: Optimizer uses index in wrong locality #36642

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

opt: Optimizer uses index in wrong locality #36642

andy-kimball opened this issue Apr 9, 2019 · 1 comment · 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 INDEX t@idx CONFIGURE ZONE USING lease_preferences = '[[+cloud=gce,+region=us-east1,+zone=us-east1-b]]'

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

EXPECTED: The query should use the t@idx index, since its lease preferences make it preferable to the primary index.

ACTUAL: The query uses the primary index.

@andy-kimball andy-kimball self-assigned this Apr 9, 2019
@andy-kimball
Copy link
Contributor Author

This bug repros when the index has constraints / lease preferences, but the table does not.

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 <tinystatemachine@gmail.com>
Co-authored-by: Andrew Kimball <andyk@cockroachlabs.com>
@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