grafana / loki

Like Prometheus, but for logs.
https://grafana.com/loki
GNU Affero General Public License v3.0
22.73k stars 3.31k forks source link

Certain queries will cause a massive memory leak when running Loki in monolith mode (v3) #13277

Open ASatanicPickle opened 1 week ago

ASatanicPickle commented 1 week ago

Describe the bug

A dramatic memory leak occurs with a specific kind of query when running the all-in-one (-target=all) loki, v3 is affected. 2.x line is fine. We had pods spike up to 100GB in a matter of minutes.

A query that causes the leak to happen:

{stream="stdout",name="loki-canary"} |= "p" | json |= "p"

These queries work fine,

{stream="stdout",name="loki-canary"} |= "p" | json
{stream="stdout",name="loki-canary"} |  json  |= "p"

Seems to be specific to having filters before and after the parser expr. (Note, I didn't test that much around this though)

I did some digging and I think the issue is here:

https://github.com/grafana/loki/blob/f8977587476169197d6da4d7055b97b189808344/pkg/logql/syntax/ast.go#L144

pkg/logql/syntax/ast.go in the reorderStages() function, specifically with the combineFilters() function.

The function seems to have a side effect where it is changing the original request's AST and with multiple queries all running in parallel, the AST gets real big. There's a heap dump below.

I was able to get it working by changing this line:

for _, s := range m { switch f := s.(type) { case *LineFilterExpr: filters = append(filters, f)

to

for _, s := range m { switch f := s.(type) { case *LineFilterExpr: filters = append(filters, MustClone(f))

Clone the filter exprs so the combineFilters() won't change the original request's AST.

To Reproduce

Steps to reproduce the behavior:

  1. Run the loki with -target=all
  2. Load it with some data ( I used the canary program)
  3. Make sure some chunks get flushed to disk, the bug will not occur if there are no chunks flushed to the store.
  4. Run the query, {stream="stdout",name="loki-canary"} |= "p" | json |= "p"

Expected behavior

Should work normally.

Environment:

The loki config that I was using locally to repro:

auth_enabled: false

server:
  http_listen_port: 3100

ingester:
    chunk_encoding: snappy
    max_chunk_age: 5m
    chunk_idle_period: 1m

common:
  instance_addr: localhost
  path_prefix: /loki
  storage:
    filesystem:
      chunks_directory: /loki/chunks
      rules_directory: /loki/rules
  replication_factor: 1
  ring:
    kvstore:
      store: inmemory

schema_config:
  configs:
    - from: 2020-10-24
      store: tsdb
      object_store: filesystem
      schema: v13
      index:
        prefix: index_
        period: 24h

ruler:
  alertmanager_url: http://localhost:9093

Heap dump while the leak was occurring:

memleak

Thanks, Mark.