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

Allocate timers outside of loops to avoid repeat allocations #4367

Merged

Commits on Feb 14, 2016

  1. Copy the full SHA
    001ef40 View commit details
    Browse the repository at this point in the history

Commits on Feb 15, 2016

  1. Allocate timers outside of loops to avoid repeat allocations

    There are currently 8 places in CockroachDB non-test code
    that create a `time.Timer` using `time.NewTimer` during
    every iteration of a loop. cockroachdb#4175 proposed a fix for the worst
    instance of this issue within `*rpcTransport.processQueue`,
    which resulted in upwards of **400,000** timers on a single node
    inuse at a given time. The second biggest offender of this
    issue was in `kv.send`, which resulted in about **30,000** timers
    on a single node inuse at a given time. Together, I diagnosed
    that these two issues were responsible for the memory "leak"
    seen in cockroachdb#4346. After making these fixes, it looks like the
    issue is gone as memory no longer grows without bound.
    
    I've gone ahead and fixed all 8 occurences of this anti-pattern
    across our codebase, using the strategy @tamird brought up in
    [this](https://github.com/cockroachdb/cockroach/pull/4175/files#r52558817)
    comment
    to avoid a race condition between iterations with the timers.
    A few of the changes might be a little over-aggressive as the
    loops are not as "tight" as the ones causing issues, but I still
    think it's important to make this change now and avoid these
    issues in the future.
    nvanbenschoten committed Feb 15, 2016
    Copy the full SHA
    89fe553 View commit details
    Browse the repository at this point in the history