Skip to content

feat(collector): use FlowSummary to reduce conntrack peak memory#30

Merged
sichenzhao merged 7 commits into
mainfrom
feat/flow-summary-memory-optimization
Mar 23, 2026
Merged

feat(collector): use FlowSummary to reduce conntrack peak memory#30
sichenzhao merged 7 commits into
mainfrom
feat/flow-summary-memory-optimization

Conversation

@sichenzhao

@sichenzhao sichenzhao commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Switches the conntrack dependency from github.com/ti-mo/conntrack to the ClickHouse fork (github.com/ClickHouse/conntrack) which provides a native FlowSummary struct and DumpFlowSummaryFilter API.
  • Uses DumpFlowSummaryFilter to dump only the fields kubenetmon needs (tuples and counters) directly from the kernel, avoiding allocation of the heavier conntrack.Flow struct with its unused fields (Status, Timeout, ProtoInfo, Labels, SeqAdj, SynProxy, etc.).
  • Updates shouldIgnoreFlow and all related code to operate on conntrack.FlowSummary instead of conntrack.Flow.
  • Vendor directory updated to reflect the dependency switch.

Mirrors ClickHouse/data-plane-application#31052 for the OSS codebase.

Test plan

  • Updated all TestShouldIgnoreFlow sub-tests to use FlowSummary
  • Existing TestCollect integration test passes (exercises the full dump → convert → send path)
  • Full test suite passes (go test ./...)

🤖 Generated with Claude Code

sichenzhao and others added 2 commits March 20, 2026 04:23
Introduce a lightweight FlowSummary struct that holds only the fields
kubenetmon actually uses (tuples and counters) from conntrack.Flow.
After each conntrack dump the full []conntrack.Flow is immediately
converted to []FlowSummary and released, allowing the GC to reclaim
the heavier Flow objects (which carry unused fields like Status,
Timeout, ProtoInfo, Labels, SeqAdj, SynProxy, etc.) before the
send loop begins.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `rawFlows = nil` assignment was flagged by golangci-lint (ineffassign)
since the variable is not read after that point. The local variable
becomes unreachable after toFlowSummaries returns, allowing the GC to
collect it without the explicit nil assignment.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sichenzhao sichenzhao requested a review from andreev-io March 20, 2026 04:39
Comment thread pkg/collector/collector.go Outdated

now := collector.clock.Now()
flows, err := collector.conntrack.Dump(&conntrack.DumpOptions{ZeroCounters: true})
rawFlows, err := collector.conntrack.Dump(&conntrack.DumpOptions{ZeroCounters: true})

@andreev-io andreev-io Mar 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for putting this up. I think we should swap in conntrack for a fork like we do internally and replace the Flow struct at that level during conversion from netlink messages. Otherwise this change may do more harm than good, since it will be allocating 3 times for each flow.

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.

updated to depend on ClickHouse forked conntrack

@sichenzhao sichenzhao requested a review from andreev-io March 21, 2026 01:10
@sichenzhao sichenzhao merged commit 6cdd092 into main Mar 23, 2026
11 of 12 checks passed
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