pyroscope icon indicating copy to clipboard operation
pyroscope copied to clipboard

feat: concurrent storage put

Open kolesnikovae opened this issue 2 years ago • 2 comments

  • [x] Make storage.Put to work concurrently
  • [x] Make sure cache GetOrCreate is thread-safe
  • [x] Add storage queue params
  • [ ] Document new params
  • ~Build tags for long ad-hoc tests~ – It may make sense to introduce a build tag for long tests (ad-hoc). As for now I adjusted the stress test so that it would run <1m.
  • ~Check if RW mutexes are good in out case~ – Experiments don't show improvements (See https://github.com/golang/go/issues/17973)

The change introduces two configuration options for the storage queue:

  • storage-queue-workers
  • storage-queue-size

On migration, especially if the deployment is large, storage throughput will potentially increate significantly, as there will be less dropped profiles, which poses some risks. We can set default parameters so that effectively writes happen serially (just one worker) to workaround this.

/cc @petethepig

kolesnikovae avatar Jul 25 '22 11:07 kolesnikovae

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
webapp/public/assets/app.js 418.12 KB (0%) 8.4 s (0%) 3.3 s (+18.72% 🔺) 11.7 s
webapp/public/assets/app.css 15.4 KB (0%) 308 ms (0%) 0 ms (+100% 🔺) 308 ms
webapp/public/assets/styles.css 9.46 KB (0%) 190 ms (0%) 0 ms (+100% 🔺) 190 ms
packages/pyroscope-flamegraph/dist/index.js 91.63 KB (0%) 1.9 s (0%) 1.7 s (+43.1% 🔺) 3.5 s
packages/pyroscope-flamegraph/dist/index.node.js 91.46 KB (0%) 1.9 s (0%) 529 ms (-4.77% 🔽) 2.4 s
packages/pyroscope-flamegraph/dist/index.css 7.05 KB (0%) 141 ms (0%) 0 ms (+100% 🔺) 141 ms

github-actions[bot] avatar Jul 25 '22 11:07 github-actions[bot]

Codecov Report

Merging #1304 (9e6aa9c) into main (1269f20) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1304   +/-   ##
=======================================
  Coverage   68.51%   68.51%           
=======================================
  Files         129      129           
  Lines        4248     4248           
  Branches     1138     1138           
=======================================
  Hits         2910     2910           
  Misses       1333     1333           
  Partials        5        5           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 25 '22 11:07 codecov[bot]

@petethepig I have updated the PR so that it won't affect existing deployments: storage queue will have one worker by default, thus effectively doing writes synchronously. Please, give it a look

kolesnikovae avatar Aug 28 '22 18:08 kolesnikovae