-
Notifications
You must be signed in to change notification settings - Fork 4k
changefeedccl: fix unwatched column families memory monitoring bug #154802
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.
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
Conversation
fb86bc0
to
3804925
Compare
445e667
to
fc8482b
Compare
} | ||
|
||
// Update watched column again to verify the feed is still progressing. | ||
// If the memory allocations leaked, this assertion would time out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to get this to fail with a clearer symptom than a timeout (at least for enterprise feeds)? This is my first time looking at the memory monitor, but my sense was that since this would set a low PerChangefeedMemLimit, that the memory monitors should fail the feed once the limit is passed. But when I tried to check that on the status of the feed in this test, I saw that it was still running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the allocations leaked, the memory monitor wouldn't fail; instead, the changefeed would get stuck, which is why the assertion would time out. I added some more explanatory comments to the test.
fc8482b
to
a14e13a
Compare
This patch fixes a bug where the memory monitor wouldn't reclaim the memory allocated to events corresponding to unwatched column families for a changefeed that targets only a subset of a table's families. Release note (bug fix): A bug where a changefeed targeting only a subset of a table's column families could become stuck has been fixed.
a14e13a
to
4f701c2
Compare
TFTRs! bors r=aerfrei,asg0451 |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. 💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details. error creating merge commit from 4f701c2 to blathers/backport-release-24.1-154802: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.1.x failed. See errors above. 💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details. error creating merge commit from 4f701c2 to blathers/backport-release-24.3-154802: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.3.x failed. See errors above. 💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details. error creating merge commit from 4f701c2 to blathers/backport-release-25.2-154802: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 25.2.x failed. See errors above. 💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details. error creating merge commit from 4f701c2 to blathers/backport-release-25.3-154802: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 25.3.x failed. See errors above. 💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This patch fixes a bug where the memory monitor wouldn't reclaim the
memory allocated to events corresponding to unwatched column families
for a changefeed that targets only a subset of a table's families.
Fixes #154776
Release note (bug fix): A bug where a changefeed targeting only a subset
of a table's column families could become stuck has been fixed.