Skip to content

Conversation

@sanki92
Copy link
Contributor

@sanki92 sanki92 commented Nov 2, 2025

Fix TestChangefeedFlushesSinkToReleaseMemory test flake

Fixes #156078

The test was failing because it encountered a grpc error "the client connection is closing" during connection teardown, which wasn't in the error allowlist. This is a normal cancellation error that should be ignored, similar to the existing "context canceled" entry.

Added the error pattern to the requireNoFeedsFail helper's ignore list.

Changed:

  • pkg/ccl/changefeedccl/helpers_test.go: Added the client connection is closing to ignoreErrs list

@sanki92 sanki92 requested a review from a team as a code owner November 2, 2025 07:18
@sanki92 sanki92 requested review from andyyang890 and removed request for a team November 2, 2025 07:18
@blathers-crl
Copy link

blathers-crl bot commented Nov 2, 2025

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added the O-community Originated from the community label Nov 2, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

`the client connection is closing`,
}
shouldIgnoreErr := func(err error) bool {
if err == nil || errors.Is(err, context.Canceled) {
Copy link
Collaborator

@andyyang890 andyyang890 Nov 4, 2025

Choose a reason for hiding this comment

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

Would adding grpcutil.IsContextCanceled(err) (or maybe grpcutil.IsClosedConnection(err)?) here work instead? I think the latter would def include this case since it checks for is closing as a substring

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

Labels

A-cdc Change Data Capture O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ccl/changefeedccl: TestChangefeedFlushesSinkToReleaseMemory failed

4 participants