Skip to content
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

[filebeat] Critical memory leak of StandardMeter in broker.Close(), the element in the global arbiter map not released #42347

Open
panbooks opened this issue Jan 20, 2025 · 1 comment
Labels
needs_team Indicates that the issue/PR needs a Team:* label

Comments

@panbooks
Copy link

panbooks commented Jan 20, 2025

  • Version: 8.17.0
  • Operating System: CentOS Linux 7

https://github.com/elastic/beats/blob/v8.17.0/libbeat/outputs/kafka/config.go#L309-L315

If adding adapter.GoMetricsNilify in newSaramaConfig when creating a broker, the object of StandardMeter in arbiter map will not be released when calling broker.Close().
In Broker.Open(), registerMeter() will create a new StandardMeter, and save it to the global arbiter map (in go-metric lib).
But the adapter.GoMetricsNilify option changes the StandardMeter to NilMeter and save it to MetricRegistry, while the StandardMeter object has already created and saved in the global map arbiter. So the original StandardMeter object will not be deleted when calling MetricRegister.Unregister or UnregisterAll.

In our production environment, through mem profile, the inuse objects and allocate objects are the same which reaches highly to 1.4 million which takes nearly 400M space, resulting in OOM when the memory is limited.

Image

Here is the benchmark to show the difference.
This code can add to here https://github.com/elastic/elastic-agent-libs/blob/v0.17.3/monitoring/adapter/go-metrics_test.go

// filters is nil
func BenchmarkGoMetricsAdapter(b *testing.B) {
 var filters []MetricFilter
 monReg := monitoring.NewRegistry()
 var reg metrics.Registry = GetGoMetrics(monReg, "test", filters...)

 for i := 0; i < b.N; i++ {
  meter := reg.GetOrRegisterAdapter("meter", func() interface{} {
   return metrics.NewMeter()
  }).(metrics.Meter)
  meter.Count()
  
  reg.UnregisterAll()
 }
}
// filters include GoMetricsNilify
func BenchmarkGoMetricsAdapterNilify(b *testing.B) {
 filters := []MetricFilter{
  GoMetricsNilify,
 }
 monReg := monitoring.NewRegistry()
 var reg metrics.Registry = GetGoMetrics(monReg, "test", filters...)

 for i := 0; i < b.N; i++ {
  meter := reg.GetOrRegister("meter", func() interface{} {
   return metrics.NewMeter()
  }).(metrics.Meter)
  meter.Count()
  
  reg.UnregisterAll()
 }
}

The BenchmarkGoMetricsAdapterNilify's result shows that the inuse objects will remain the same with allocated objects.
But the BenchmarkGoMetricsAdapter's result shows that the inuse objects remain zero.

@panbooks panbooks changed the title [filebeat] Critical memory leak of StandardMeter in broker.Close(), the element int arbiter map not released [filebeat] Critical memory leak of StandardMeter in broker.Close(), the element in the global arbiter map not released Jan 20, 2025
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 20, 2025
@botelastic
Copy link

botelastic bot commented Jan 20, 2025

This issue doesn't have a Team:<team> label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_team Indicates that the issue/PR needs a Team:* label
Projects
None yet
Development

No branches or pull requests

1 participant