From c7c50ed74f5f22355d9fe173e680ec5f3bcfffbf Mon Sep 17 00:00:00 2001 From: Marc Lopez Rubio Date: Thu, 24 Mar 2022 08:53:14 +0100 Subject: [PATCH 1/2] modelindexer: Reduce locking on flushActive This patch unlocks the `i.activeMu` mutex in flushActive as soon as the `i.active` reference has been set to `nil`. This severely minimizes the lock contention and achives similar or higher indexing throughput when comparing the benchmarks before #7352 was introduced. These are the results: ```console $ go test -bench ... # Current main goos: darwin goarch: arm64 pkg: github.com/elastic/apm-server/model/modelindexer BenchmarkModelIndexer/NoCompression-8 4653790 2527 ns/op BenchmarkModelIndexer/BestSpeed-8 2909288 4051 ns/op BenchmarkModelIndexer/DefaultCompression-8 1691677 6674 ns/op BenchmarkModelIndexer/BestCompression-8 1234953 8334 ns/op PASS ok github.com/elastic/apm-server/model/modelindexer 70.585s ``` ```console $ go test -bench ... # This patch goos: darwin goarch: arm64 pkg: github.com/elastic/apm-server/model/modelindexer BenchmarkModelIndexer/NoCompression-8 8702388 1344 ns/op BenchmarkModelIndexer/BestSpeed-8 5097385 2238 ns/op BenchmarkModelIndexer/DefaultCompression-8 2639126 4821 ns/op BenchmarkModelIndexer/BestCompression-8 1586126 7350 ns/op PASS ok github.com/elastic/apm-server/model/modelindexer 64.933s ``` The contention is much worse when the APM Server is actually running and indexing against an Elasticsearch cluster since the lock is held for the entire `bulkIndexer.Flush` operation, which includes network latency. Signed-off-by: Marc Lopez Rubio --- model/modelindexer/indexer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/modelindexer/indexer.go b/model/modelindexer/indexer.go index 4fb0ae70231..a0b12b88bdf 100644 --- a/model/modelindexer/indexer.go +++ b/model/modelindexer/indexer.go @@ -334,9 +334,9 @@ func (i *Indexer) flushActive(ctx context.Context) error { }() i.activeMu.Lock() - defer i.activeMu.Unlock() bulkIndexer := i.active i.active = nil + i.activeMu.Unlock() err := i.flush(ctx, bulkIndexer) bulkIndexer.Reset() i.available <- bulkIndexer From 53b2d463eba89fc443e6249452d258b45ba9da2c Mon Sep 17 00:00:00 2001 From: Marc Lopez Rubio Date: Thu, 24 Mar 2022 10:24:37 +0100 Subject: [PATCH 2/2] Update changelog Signed-off-by: Marc Lopez Rubio --- changelogs/head.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/changelogs/head.asciidoc b/changelogs/head.asciidoc index 0a98131034d..4787d500741 100644 --- a/changelogs/head.asciidoc +++ b/changelogs/head.asciidoc @@ -13,6 +13,7 @@ https://github.com/elastic/apm-server/compare/8.1\...main[View commits] [float] ==== Bug fixes - Do not overwrite `service version` if no transaction/error/... specific `service.name` is given {pull}7281[7281] +- modelindexer: Fix indexing performance regression due to locking bug {pull}7649[7649] [float] ==== Intake API Changes