search

sourcegraph / appdash

Application tracing system for Go, based on Google's Dapper.
https://sourcegraph.com
Other
1.72k stars 137 forks source link

add a FlushTimeout field to ChunkedCollector to prevent memory consumption #113

Closed slimsag closed 8 years ago

slimsag commented 8 years ago

This change adds a FlushTimeout field to ChunkedCollector which can be used to prevent a large queue from building up within the ChunkedCollector, effectively leaking/consuming large amounts of memory, due to an misbehaving underlying collector (like RemoteCollector, which would attempt to reconnect to a flaky remote server upon each collection).

/cc @neelance @keegancsmith

dmitshur commented 8 years ago

LGTM, but see comments.

keegancsmith commented 8 years ago

LGTM

slimsag commented 8 years ago

@shurcooL could you please review the new changes? I've added an upper bound to the queue size based on an (est.) amount of RAM usage per feedback from @neelance

In the process I've also clarified a lot of the documentation and made it more concrete, in addition to adding a NewChunkedCollector method which picks the (generally) right defaults.

dmitshur commented 8 years ago

I've reviewed the changes since https://github.com/sourcegraph/appdash/pull/113#issuecomment-190560933, and you've addressed all my new comments. LGTM.