Skip to content

release-21.2: kvserver: scan only intents during rts scan#72315

Merged
stevendanna merged 1 commit intocockroachdb:release-21.2from
stevendanna:backport21.2-71295
Nov 18, 2021
Merged

release-21.2: kvserver: scan only intents during rts scan#72315
stevendanna merged 1 commit intocockroachdb:release-21.2from
stevendanna:backport21.2-71295

Conversation

@stevendanna
Copy link
Copy Markdown
Collaborator

@stevendanna stevendanna commented Nov 2, 2021

Backport 1/1 commits from #71295 and an additional commit
to gate the new behaviour behind a cluster setting, per our
backport policy.

/cc @cockroachdb/release

In 21.2, separated intents are the default. Once migrated, we can then
use this to iterate over substantially less data to find all intents
for a given keyspan. The hope is that this will make rangefeed
start-up substantially cheaper.

Informs #70920
Fixes #69697

Release note: None

Release justification: Performance improvement to help address the impact
of CHANGEFEED restarts.

@stevendanna stevendanna requested a review from a team as a code owner November 2, 2021 10:37
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Nov 2, 2021 •
edited by stevendanna
Loading

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Nov 2, 2021

This change is Reviewable

@stevendanna
kvserver: scan only intents during rangefeed rts scan
7e95279 
In 21.2, seperated intents are the default. Once migrated, we can then
use this to iterate over substantially less data to find all intents
for a given keyspan. The hope is that this will make rangefeed
start-up substantially cheaper.

Release note: None
@stevendanna stevendanna self-assigned this Nov 18, 2021
@stevendanna stevendanna merged commit a4bbd5b into cockroachdb:release-21.2 Nov 18, 2021
stevendanna added a commit to stevendanna/cockroach that referenced this pull request Mar 22, 2022
@stevendanna
settings: retire kv.rangefeed.separated_intent_scan.enabled
3b9dfdc 
This setting was added in a 21.2 backport

cockroachdb#72315

but was never added to master:

cockroachdb#71295

So from the perspective of someone who upgrades from 21.2, it is now a
retired setting.

Release justification: Low-risk bug fix

Release note: None
@stevendanna stevendanna mentioned this pull request Mar 22, 2022
Merged
craig bot pushed a commit that referenced this pull request Mar 22, 2022
@jocrl @adityamaru @stevendanna
Merge #78186 #78213 #78243 #78246
e1e6f39 
78186: ui: improve UI for statuses on the jobs page r=jocrl a=jocrl

Addresses #71963

Previously the `cancel-requested`, `pause-requested`, and `revert-failed` had
blue badges.
![image](https://user-images.githubusercontent.com/91907326/138773770-84aa4637-d87c-47a2-bd07-7d02c2322982.png) 

Now, `cancel-requested` and `pause-requested` have gray badges and
`revert-failed` has a red badge.
![image](https://user-images.githubusercontent.com/91907326/159310950-9742cc1b-9350-4c6d-990b-3e7e2197e681.png)

Release note (ui): Improved colors for status badges on the Jobs page. Three
status on the Jobs page, `cancel-requested`, `pause-requested`, and
`revert-failed`, previously had blue status badge colors that were
uninformative of their meaning. This commit modifies the badge colors to
reflect their meaning. Now `cancel-requested` and `pause-requested` have gray
badges and `revert-failed` has a red badge.

78213: distsql: simple projection in experimental distsql planner panics r=msirek a=msirek

Previously, selecting a given column from a table more than once could
cause an `index out of range` panic when experimental_distsql_planning
is set to always, for example:
```
CREATE TABLE kv (k INT PRIMARY KEY, v INT);
INSERT INTO kv VALUES (1, 1), (2, 1), (3, 2);
SET experimental_distsql_planning = always;
SELECT v, k, k, v FROM kv;
```
This commit fixes the issue, which is due to incorrect mapping of
selected columns to source column ordinals in
`ConstructSimpleProject`.

Release note: none

78243: backupccl: fix paper cut in latest files directory structure r=DarrylWong a=adityamaru

Prior to this change, the LATEST files of a backup were written to
`metadatalatest/` instead of `metadata/latest/`, this patch fixes
that.

Release note (bug fix): The LATEST file that points to the latest full
backup in a collection was written to a directory path with the wrong
structure.

78246: settings: retire kv.rangefeed.separated_intent_scan.enabled  r=nvanbenschoten a=stevendanna

This setting was added in a 21.2 backport

#72315

but was never added to master:

#71295

So from the perspective of someone who upgrades from 21.2, it is now a
retired setting.

Release justification: Low-risk bug fix

Release note: None

Co-authored-by: Josephine Lee <josephine@cockroachlabs.com>
Co-authored-by: Mark Sirek <sirek@cockroachlabs.com>
Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
blathers-crl bot pushed a commit that referenced this pull request Mar 22, 2022
@stevendanna
settings: retire kv.rangefeed.separated_intent_scan.enabled
55d3627 
This setting was added in a 21.2 backport

#72315

but was never added to master:

#71295

So from the perspective of someone who upgrades from 21.2, it is now a
retired setting.

Release justification: Low-risk bug fix

Release note: None
@blathers-crl blathers-crl bot mentioned this pull request Mar 22, 2022
Merged
stevendanna added a commit that referenced this pull request Jun 6, 2022
@stevendanna
settings: retire kv.rangefeed.separated_intent_scan.enabled
7545b6d 
This setting was added in a 21.2 backport

#72315

but was never added to master:

#71295

So from the perspective of someone who upgrades from 21.2, it is now a
retired setting.

Release justification: Low-risk bug fix

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

Reviewers

1 more reviewer

@nvb nvb nvb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@stevendanna stevendanna

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stevendanna @cockroach-teamcity @nvb