Skip to content

Conversation

@petrosagg
Copy link
Contributor

@petrosagg petrosagg commented Jan 14, 2025

This is a slight oversight in #30858 that switched from the async operator builder that drops capabilities when it exits to the timely rc operator builder where capabilities need to dropped manually.

Motivation

  • This PR fixes a recognized bug.

Fixes https://github.com/MaterializeInc/database-issues/issues/8880

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

This is a slight oversight in MaterializeInc#30858 that switched from the async
operator builder that drops capabilities when it exits to the timely rc
operator builder where capabilities need to dropped manually.

Signed-off-by: Petros Angelatos <[email protected]>
@petrosagg petrosagg requested a review from a team as a code owner January 14, 2025 16:42
@petrosagg petrosagg requested a review from teskje January 14, 2025 16:42
@def-
Copy link
Contributor

def- commented Jan 14, 2025

What did you have to do to make the memory leak quickly? I want to add a regression test for this.

@petrosagg
Copy link
Contributor Author

What did you have to do to make the memory leak quickly? I want to add a regression test for this.

I just logged the number of running dataflows in timely and saw that the old ones weren't being cleaned up. It's going to be very tricky to write a regression test with the codebase as is. If this were a compute cluster we could have used the introspection views to verify that dataflows get cleaned up but storage clusters don't have this ability.

@jkosh44 jkosh44 merged commit 4294ebd into MaterializeInc:main Jan 14, 2025
80 checks passed
jkosh44 pushed a commit that referenced this pull request Jan 14, 2025
This is a slight oversight in #30858 that switched from the async
operator builder that drops capabilities when it exits to the timely rc
operator builder where capabilities need to dropped manually.

Signed-off-by: Petros Angelatos <[email protected]>
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.

4 participants