Skip to content

changefdeedccl: Unskip alter changefeed tests#106951

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
miretskiy:flakes
Jul 18, 2023
Merged

changefdeedccl: Unskip alter changefeed tests#106951
craig[bot] merged 1 commit intocockroachdb:masterfrom
miretskiy:flakes

Conversation

@miretskiy
Copy link
Copy Markdown
Contributor

Fixes #83946

Unskip previously skipped tests.
TestAlterChangefeedInitialScan is no longer flaky. Fix TestAlterChangefeedDatabaseQualifiedNames -- the flakiness was due to the incorrect logic in test feed machinery that would erroneously skip data file.
Remove TestAlterChangefeedUpdateFilter test -- as the comment says it's a test for now removed/deprecated primary_key_filter option (replaced with cdc queries). Ability to alter changefeeds with cdc query is currently on a backlog and the test will need to be re-written enterely to support that.

Release note: None

@miretskiy miretskiy requested a review from a team as a code owner July 17, 2023 21:17
@miretskiy miretskiy requested review from samiskin and removed request for a team July 17, 2023 21:17
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@miretskiy miretskiy requested review from jayshrivastava and removed request for samiskin July 17, 2023 21:18

// Add implements Writer interface.
func (b *blockingBuffer) Add(ctx context.Context, e Event) error {
if log.V(1) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe use V(2) here bc it's per event?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

Value: tree.NewStrVal(string(changefeedbase.OptFormatParquet)),
},
)
log.Infof(context.Background(), "using parquet format; NAH") // DO NOT MERGE
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember to uncomment before merging :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah! Thanks!

Fixes cockroachdb#83946

Unskip previously skipped tests.
TestAlterChangefeedInitialScan is no longer flaky.
Fix TestAlterChangefeedDatabaseQualifiedNames -- the flakiness
was due to the incorrect logic in test feed machinery
that would erroneously skip data file.
Remove TestAlterChangefeedUpdateFilter test -- as the comment says
it's a test for now removed/deprecated primary_key_filter option
(replaced with cdc queries).  Ability to alter changefeeds with
cdc query is currently on a backlog and the test will need to be
re-written enterely to support that.

Release note: None
@miretskiy
Copy link
Copy Markdown
Contributor Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 18, 2023

Build succeeded:

@craig craig bot merged commit a06ea31 into cockroachdb:master Jul 18, 2023
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 this pull request may close these issues.

changefeedccl: Changefeed tests became excessively flaky

3 participants