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

release-23.1.0: TODO #101391

Merged
merged 3 commits into from
Apr 13, 2023
Merged

release-23.1.0: TODO #101391

merged 3 commits into from
Apr 13, 2023

Conversation

miretskiy
Copy link
Contributor

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

Release justification: Usability fixes

Yevgeniy Miretskiy added 3 commits April 12, 2023 18:53
changefeedccl: Encoding and cdc_prev handling fixes
12a0b3d 
Fix multiple related handling and encoding issues
when using CDC queries.

Assuming the following changefeed:
`CREATE CHANGEFEED ... AS SELECT *, cdc_prev, event_op() FROM t`

* Newly inserted rows will contain NULL in the `cdc_prev` column.
* Deleted rows will emit all *primary key* columns; all other columns
  will be left `NULL`.  `event_op` column will contain appropriate even
  op description (`deleted`)
* `cdc_prev` tuple no longer contains cdc_mvcc_internal_timestamp column
  This was a bug that incorrectly made that column available in
  cdc_prev.  If the application needs to determine how much time
  exlapsed since the previous update, the application should update an
  explicit timestamp column, and use that instead of the system column.

The deleted row handling, when using CDC Queries, might be a bit awkward
in that the row is still emitted, albeit with all but the primary key
columns set to `NULL`.  Of course, there is an `event_op()` function
that can make the distinction between a new row with `NULL` values, or
a deleted row.  However, the customers may find `wrapped` envelope
simpler to use (`WITH envelope='wrapped', format='json', diff`) would produce
JSON object with `before` and `after` keys containing prior/current
state of the row).  This PR makes it possible to use `wrapped` envelope
with `diff` option when using CDC queries.  The `before` value in the
output will always be an entire row -- without any projections.

Fixes cockroachdb#101000

Release note (enterprise change): CDC queries now support wrapped
envelope with diff (`envelope='wrapped', diff`).
changefeedccl: Remove skipped tests that decayed over time
4a1ec78 
Remove
Fixes cockroachdb#32232
Remove TestChangefeedNodeShutdown.  This test has been disabled since
2018; Other tests exist (e.g. `TestChangefeedHandlesDrainingNodes`) that verify
restart behavior.

Fixes cockroachdb#51842
Remove BenchmarkChangefeedTicks benchmark.  This benchmark has been
skipped since 2019.  Attempts could be made to revive it; however, this
benchmark had a lot of code, which accomplished questionable goals.
The benchmark itself was unrepresentative (by using dependency
injection), too small to be meaningful (1000 rows), and most likely
would be too noise and inconclusive.  We have added other micro
benchmarks over time; and we conduct large scale testing, including
with roachtests.

Release note: None
changefeedccl: Unskip TestChangefeedJobUpdateFailsIfNotClaimed test
51bf333 
Fixes cockroachdb#91548

Release note: None
@miretskiy miretskiy requested a review from a team as a code owner April 12, 2023 22:53
@miretskiy miretskiy requested review from jayshrivastava and removed request for a team April 12, 2023 22:53
@blathers-crl
Copy link

blathers-crl bot commented Apr 12, 2023

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
Member

This change is Reviewable

@miretskiy miretskiy requested a review from HonoreDB April 12, 2023 22:54
@miretskiy miretskiy merged commit 3c86264 into cockroachdb:release-23.1.0 Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HonoreDB HonoreDB HonoreDB approved these changes

@jayshrivastava jayshrivastava Awaiting requested review from jayshrivastava jayshrivastava was automatically assigned from cockroachdb/cdc-prs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@miretskiy @cockroach-teamcity @HonoreDB