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

add writeQueue buffer to memoryIdx #1365

Merged
merged 4 commits into from
Jun 26, 2019
Merged

add writeQueue buffer to memoryIdx #1365

merged 4 commits into from
Jun 26, 2019

Conversation

woodsaj
Copy link
Member

@woodsaj woodsaj commented Jun 24, 2019

Add support for a writeQueue that holds new MetricDefinitions in a
buffer, then writes them into the index as a single batch.

issue #1353
see also: #1152 (probably fixes)

@woodsaj woodsaj force-pushed the asyncAddToIdx branch 2 times, most recently from 7b19cb3 to 9de455c Compare June 24, 2019 21:30
Add support for a writeQueue  that holds new MetricDefinitions in a
buffer, then writes them into the index as a single batch.
Copy link
Contributor

@robert-milan robert-milan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, I haven't run the tests on my system yet.

idx/memory/memory.go Outdated Show resolved Hide resolved
idx/memory/write_queue.go Show resolved Hide resolved
idx/memory/memory.go Outdated Show resolved Hide resolved
@replay
Copy link
Contributor

replay commented Jun 25, 2019

Looks good to me, except the things that robert already commented

- fix races caused by using atomic operations only some of the time.
  The most common issue was dereferencing *idx.Archive structs.  This
  is unsafe as the process of defeferencing requires reading the
  values of the struct fields.  However we use atomic operations
  to update some fields.  The fix is to use our own CloneArchive
  function to safely dereference an *idx.Archive.
- handle races where archives are flushed from the writeQueue after
  we fail to find the archive in the index, but before we check if
  they are in the writeQueue.
idx/memory/memory.go Outdated Show resolved Hide resolved
@Dieterbe
Copy link
Contributor

some stats could be useful?

@woodsaj
Copy link
Member Author

woodsaj commented Jun 26, 2019

This branch has been thoroughly tested with a load-simulator.

Stats from Master:

MetricAdd:
metric-add.latency.min.gauge32 1 1561571237
metric-add.latency.mean.gauge32 9 1561571237
metric-add.latency.median.gauge32 1 1561571237
metric-add.latency.p75.gauge32 1 1561571237
metric-add.latency.p90.gauge32 1 1561571237
metric-add.latency.max.gauge32 29999 1561571237
metric-add.values.count32 4559 1561571237
metric-add.values.rate32 44.870178803035756 1561571237

MetricUpdate:
metric-update.latency.min.gauge32 1 1561571237
metric-update.latency.mean.gauge32 0 1561571237
metric-update.latency.median.gauge32 1 1561571237
metric-update.latency.p75.gauge32 1 1561571237
metric-update.latency.p90.gauge32 1 1561571237
metric-update.latency.max.gauge32 1000 1561571237
metric-update.values.count32 223279 1561571237
metric-update.values.rate32 2197.5364897731506 1561571237

Query:
query.latency.min.gauge32 1 1561571237
query.latency.mean.gauge32 3008 1561571237
query.latency.median.gauge32 1 1561571237
query.latency.p75.gauge32 100 1561571237
query.latency.p90.gauge32 29999 1561571237
query.latency.max.gauge32 29999 1561571237
query.values.count32 14366 1561571237
query.values.rate32 141.39175499508622 1561571237

stats from this Branch

MetricAdd:
metric-add.latency.min.gauge32 1 1561572495
metric-add.latency.mean.gauge32 0 1561572495
metric-add.latency.median.gauge32 1 1561572495
metric-add.latency.p75.gauge32 1 1561572495
metric-add.latency.p90.gauge32 1 1561572495
metric-add.latency.max.gauge32 50 1561572495
metric-add.values.count32 12406 1561572495
metric-add.values.rate32 120.45101479214206 1561572495

MetricUpdate:
metric-update.latency.min.gauge32 1 1561572495
metric-update.latency.mean.gauge32 0 1561572495
metric-update.latency.median.gauge32 1 1561572495
metric-update.latency.p75.gauge32 1 1561572495
metric-update.latency.p90.gauge32 1 1561572495
metric-update.latency.max.gauge32 65 1561572495
metric-update.values.count32 569688 1561572495
metric-update.values.rate32 5531.15412542331 1561572495

Query:
query.latency.min.gauge32 1 1561572495
query.latency.mean.gauge32 305 1561572495
query.latency.median.gauge32 1 1561572495
query.latency.p75.gauge32 1 1561572495
query.latency.p90.gauge32 7 1561572495
query.latency.max.gauge32 29999 1561572495
query.values.count32 13401 1561572495
query.values.rate32 130.1115663551759 1561572495

as well as ingesting data 2x as fast, the p90 query latency dropped from 30s to 7ms.

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.

None yet

4 participants