Skip to content

[ISSUE #542] Fix connect runtime stats oom - #543

Open
VictoryAnn wants to merge 1 commit into
apache:masterfrom
VictoryAnn:dev
Open

[ISSUE #542] Fix connect runtime stats oom#543
VictoryAnn wants to merge 1 commit into
apache:masterfrom
VictoryAnn:dev

Conversation

@VictoryAnn

Copy link
Copy Markdown

What is the purpose of the change

fix connect runtime stats bug.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review: Approved ✅

PR: #543 — [ISSUE #542] Fix connect runtime stats OOM
Type: Bug fix (1 file, +2/-2)

Assessment

Fixes OutOfMemoryError in connect runtime statistics collection. Minimal 2-line change.

Verdict

✅ Correct fix for memory issue in stats service.


🤖 Automated review by oss-sentinel-ai

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — clear copy-paste bug fix. The size check and removal were operating on sourceTaskTimesList instead of sinkTaskTimesList, causing the sink list to grow unboundedly and eventually OOM. Fix is correct.


Automated review by github-manager-bot

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

Correct bug fix for a copy-paste error causing OOM in ConnectStatsService.sampling().

Findings

  • [Critical] The original code was checking sourceTaskTimesList.size() and removing from sourceTaskTimesList immediately after adding to sinkTaskTimesList. This meant sinkTaskTimesList grew unboundedly (OOM), while sourceTaskTimesList was incorrectly trimmed after every sampling cycle.
  • [Info] The fix correctly changes both the size check and the removeFirst() call to operate on sinkTaskTimesList, matching the list that was just appended to on the line above.

Verdict

LGTM. Clean fix for a clear copy-paste bug.

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.

2 participants