Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

this might fix an OOM problem in cache.searchForward #696

Merged
merged 1 commit into from
Jul 24, 2017
Merged

Conversation

Dieterbe
Copy link
Contributor

No description provided.

@Dieterbe Dieterbe force-pushed the maybe-oom-fix branch 5 times, most recently from 22558a8 to b9d4df9 Compare July 23, 2017 03:00
with the following symptoms:

```
Showing nodes accounting for 8385.53MB, 98.50% of 8513.19MB total
Dropped 217 nodes (cum <= 42.57MB)
Showing top 30 nodes out of 60
      flat  flat%   sum%        cum   cum%
 5171.26MB 60.74% 60.74%  5173.26MB 60.77%  github.com/raintank/metrictank/mdata/cache.(*CCacheMetric).searchForward /home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/mdata/cache/ccache_metric.go
  719.10MB  8.45% 69.19%   986.11MB 11.58%  github.com/raintank/metrictank/mdata.NewAggMetric /home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/mdata/aggmetric.go
  714.62MB  8.39% 77.59%   714.62MB  8.39%  runtime.rawstringtmp /usr/local/go/src/runtime/string.go
  616.55MB  7.24% 84.83%   617.55MB  7.25%  github.com/raintank/metrictank/mdata/chunk.New /home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/mdata/chunk/chunk.go
  436.67MB  5.13% 89.96%   436.67MB  5.13%  github.com/raintank/metrictank/vendor/github.com/dgryski/go-tsz.(*bstream).writeBits /home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/vendor/github.com/dgryski/go-tsz/bstream.go
...
         .          .    194:    // add all consecutive chunks to search results, starting at the one containing "from"
         .          .    195:    for ; ts != 0; ts = mc.chunks[ts].Next {
         .        2MB    196:        log.Debug("CCacheMetric searchForward: forward search adds chunk ts %d to start", ts)
    5.05GB     5.05GB    197:        res.Start = append(res.Start, mc.chunks[ts].Itgen)
...
```
@Dieterbe Dieterbe changed the title this MIGHT fix it this might fix an OOM problem in cache.searchForward Jul 23, 2017
@replay
Copy link
Contributor

replay commented Jul 23, 2017

If we would end up in a situation where mc.chunks[ts].Next == ts then that would be a huge problem and end in an infinite loop as you said.
If this is the case then the hardest part will be figuring out how it got into that state, which it should never be in.

@replay
Copy link
Contributor

replay commented Jul 23, 2017

so I think deploying it wouldn't hurt, I hope that it won't detect such an inconsistency

@replay replay self-requested a review July 23, 2017 10:36
@Dieterbe Dieterbe merged commit bcb3472 into master Jul 24, 2017
@Dieterbe Dieterbe deleted the maybe-oom-fix branch September 18, 2018 09:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants