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.

Sign up for GitHub

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

Jump to bottom

opt: use the catalog for access to tenant zone configs, for locality-aware planning #70558

Closed
irfansharif opened this issue Sep 22, 2021 · 1 comment
Closed

opt: use the catalog for access to tenant zone configs, for locality-aware planning #70558

irfansharif opened this issue Sep 22, 2021 · 1 comment
Assignees
@ajwerner
Labels
A-sql-optimizer SQL logical planning and optimizations. A-zone-configs C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Sep 22, 2021 •
edited by ajwerner
Loading

#66348 outlined a scheme to support zone configs for secondary tenants, the MVP for which we implemented as part of #67679. One part left out of the MVP was replacing our use of the system config span in the optimizer (which is the only remaining blocker for enabling locality-aware planning for secondary tenants).

The optimizer uses zone configs to generate locality optimized plans. Today it uses the gossiped SystemConfigSpan data to access the descriptor's (potentially stale) zone config.
(a) This information only available for the host tenant, and
(b) We want to stop gossiping the system config span (see the RFC).

To continue providing the optimizer access to zone configs, we could cache them in the catalog layer and associate them with their descriptors.

Epic: CRDB-10489
@blathers-crl
Copy link

blathers-crl bot commented Sep 22, 2021

Hi @irfansharif, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@irfansharif irfansharif added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-optimizer SQL logical planning and optimizations. A-zone-configs labels Sep 22, 2021
@irfansharif irfansharif mentioned this issue Dec 15, 2021
Closed
@irfansharif irfansharif mentioned this issue Dec 15, 2021
Closed
@ajwerner ajwerner self-assigned this Jan 11, 2022
@exalate-issue-sync exalate-issue-sync bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jan 11, 2022
ajwerner added a commit to ajwerner/cockroach that referenced this issue Jan 13, 2022
@ajwerner
server/systemconfigwatcher: use rangefeeds to watch the system config
484b405 
This also adopts the rangefeed backed cache for the optimizer and for
the reporter.

Fixes cockroachdb#70558
Fixes cockroachdb#74665

Release note: None
ajwerner added a commit to ajwerner/cockroach that referenced this issue Jan 31, 2022
@ajwerner
server/systemconfigwatcher: use rangefeeds to watch the system config
aeb377a 
This also adopts the rangefeed backed cache for the optimizer and for
the reporter.

Fixes cockroachdb#70558
Fixes cockroachdb#74665

Release note: None
@craig craig bot closed this as completed in 9d9e975 Feb 2, 2022
@ajwerner ajwerner mentioned this issue Feb 2, 2022
Closed
@arulajmani arulajmani mentioned this issue Feb 2, 2022
Merged
arulajmani added a commit to arulajmani/cockroach that referenced this issue Feb 2, 2022
@arulajmani
logictestccl: fix stale issue number in TODO
32db28a 
We closed cockroachdb#69265 in favour of cockroachdb#70558, and the only remaining work
left to address locality aware planning for tenants is captured
in cockroachdb#75864.

Release note: None
craig bot pushed a commit that referenced this issue Feb 2, 2022
@sumeerbhola @aayushshah15
@jeffswenson @rafiss @RichardJCai @nicktrav @stevendanna @rail @arulajmani
Merge #58261 #75746 #75766 #75804 #75822 #75843 #75845 #75857 #75865 #…
88522a3 
…75871

58261: opt,sql: use paired-joins with non-covering indexes for left joins r=rytaft a=sumeerbhola

This is done when the left outer/semi/anti join can use a
lookup join. Prior to this, when the non-covering index
could not fully evaluate the filter for left join we could
not generate a lookup join.

With this change:
- Left outer join becomes a pair of two left outer joins.
- Left semi join is a pair of inner join followed by left
  semi join.
- Left anti join is a pair of left outer join followed by
  left anti join.

Informs #55452

Release note (performance improvement): The optimizer can now
generate lookup joins in certain cases for non-covering
indexes, when performing a left outer/semi/anti join.

75746: dev: initialize submodules in `dev doctor` r=aayushshah15 a=aayushshah15

This commit adds a check to `dev doctor` to initialize submodules, like we do
in our `Makefile`.

Fixes #72247

Release note: None

75766: server: do not check decommission list for the tenant r=JeffSwenson a=JeffSwenson

Previously, the system tenant would return PermissionDenied if the
tenant's instance_id was equivalent to a decommissioned node's id.

Now, the system tenant does not check the decommissioned node list if
the incoming node_id belongs to a non-system tenant.

This PR feeds the request context down to the OnOutgoingPing and
OnIncomingPing callbacks. Previously the callbacks were using the
ambient context. The only use of the context was a storage.MVCCGet call
in nodeTombstoneStorage.IsDecommissioned.

Release note: None

75804: sql: support RESET ALL statement r=otan a=rafiss

fixes #75435

Release note (sql change): Support for the RESET ALL statement was
added. This statement resets the values of all session variables to
their default values.

75822: sql: error when setting timezone outside of postgres max utc offsets r=otan a=RichardJCai

Release note (sql change): Previously, users would be able to set
a UTC timezone offset of greater than 167 or less than -167. This
now returns an error.

Example:

SET TIME ZONE '168'
Gives error:
invalid value for parameter "timezone": "'168'": cannot find time zone "168": UTC timezone offset is out of range.

SET TIME ZONE '-168'
Gives error:
invalid value for parameter "timezone": "'-168'": cannot find time zone "-168": UTC timezone offset is out of range.

Fixes #75168

Note: If a user has already set a UTC timezone offset outside of these bounds, it will be unchanged. 

75843: c-deps/krb5: fix build for more recent versions of autoconf r=otan a=nicktrav

More recent versions of `autoconf`, when used to build `krb5`, generates
shell scripts with invalid syntax.

Fix by pulling in the [upstream patch][1] for the issue into our tree.

Closes #72529.

[1]:
krb5/krb5@f78edbe

75845: vendor: bump Pebble to 38b68e17aa97 r=jbowens a=nicktrav

Pebble commits:

```
38b68e17 internal/batchskl: return error on index overflow
8440f290 internal/manifest: use a line sweep to optimize NewL0Sublevels
0f5acb26 sstable: add direct block reading to suffix rewriter
26856d10 db: avoid stats flake in TestMemTableReservation
b452808f sstable: Make sstable Writer.Close idempotent
17fe1a65 sstable: add RewriteKeySuffixes function
c9e6edfc db: expose metrics on count and earliest seqnum of snapshots
b958d9a7 sstable: add a writeQueue to the sstable writer
c8dad06c db: disable automatic compactions in `MetricsTest`
015f5e38 internal/rangekey: fix range key iteration bug
```

The commit `38b68e17` contains the fix for #69906.

Closes #69906.

Release note: none

75857: sql: fix small race in distIndexBackfiller r=adityamaru a=stevendanna

This fixes a small race condition in
distIndexBackfiller. updateJobDetails calls SetResumeSpansInJob which
mutates the ResumeSpanList in the job details.  Normally, this is only
called from the periodic updater.  However, when the testing knob
AlwasyUpdateIndexBackfillDetails is set, we also update it on every
ProducerMetadata message we get back

Release note: None

75865: build: address util.log.logcrash package rename r=knz a=rail

After `util.log` was renamed to `util.log.logcrash`, the build system
stopped updating the Sentry environment variable properly. Instead of
setting it to the release version, it was falling back to the default
"development" value. As a result, all Sentry reports went to the
development environment bucket.

This patch addresses the name change.

Release note: None

75871: logictestccl: fix stale issue number in TODO r=arulajmani a=arulajmani

We closed #69265 in favour of #70558, and the only remaining work
left to address locality aware planning for tenants is captured
in #75864.

Release note: None

Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Aayush Shah <[email protected]>
Co-authored-by: Jeff <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: richardjcai <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: arulajmani <[email protected]>
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ajwerner ajwerner

Labels
A-sql-optimizer SQL logical planning and optimizations. A-zone-configs C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ajwerner @irfansharif