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

improve kafka-mdm stats/priority #1200

Merged
merged 12 commits into from
Jan 21, 2019
Merged

improve kafka-mdm stats/priority #1200

merged 12 commits into from
Jan 21, 2019

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Jan 2, 2019

cleaning up the code a bit in prep for #1022

TODO: same for metricpersist (maybe in a separate PR)

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Jan 3, 2019

i ran benchmarks to try to find a performance impact, but there is none. only noise.
this is
fakemetrics backfill --kafka-mdm-addr localhost:9092 --offset 22h --period 10s --speedup 50 --mpo 20000 + docker-dev-custom-cfg-kafka + for every commit, compiling MT and restarting the container to measure ingest speed, using a modified dashboard that also shows movinaverage of 9minutes of ingest data, to boil down the bench run to a single number.
the main reason i wanted to check performance is because of i thought there could be false sharing between multiple goroutines accessing the same memory pages because of the stats.
(note for some of the runs, there's no kafka lag data because of a bug)

https://snapshot.raintank.io/dashboard/snapshot/Q523wHqQ21IwSp9BygfxmSr34DwTxW9x?orgId=2
bench-log.txt (see bottom for avg ingest rate)

https://snapshot.raintank.io/dashboard/snapshot/FrXPzkYC6zGRTJJE7Eq4D8ECdkjArnvb?orgId=2
bench-log-2.txt (see bottom for avg ingest rate)

for completeness, i'll do another run with padded stats structs (to rule out false sharing), once i have time to leave my computer unattended.

@Dieterbe Dieterbe changed the title WIP: improve kafka-mdm stats/priority improve kafka-mdm stats/priority Jan 3, 2019
input/kafkamdm/kafkamdm.go Outdated Show resolved Hide resolved
@Dieterbe
Copy link
Contributor Author

Dieterbe commented Jan 3, 2019

for completeness, i'll do another run with padded stats structs (to rule out false sharing), once i have time to leave my computer unattended.

diff --git a/input/kafkamdm/kafkamdm.go b/input/kafkamdm/kafkamdm.go
index b0398b18..585a569d 100644
--- a/input/kafkamdm/kafkamdm.go
+++ b/input/kafkamdm/kafkamdm.go
@@ -177,6 +177,7 @@ type KafkaStats struct {
        offset  stats.Gauge64
        logSize stats.Gauge64
        lag     stats.Gauge64
+       _p1     [5]uint64
 }
 

-> no gains

input/kafkamdm/kafkamdm.go Outdated Show resolved Hide resolved
partitionOffsetMetric := partitionOffset[partition]
partitionLogSizeMetric := partitionLogSize[partition]
partitionLagMetric := partitionLag[partition]
kafkaStats := kafkaStats[partition]
Copy link
Contributor

Choose a reason for hiding this comment

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

detail: this could be initialized further down on :274 so we can save the allocation if there's an error in getting the offset

Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

LGTM with one last comment

@Dieterbe Dieterbe merged commit b1c2c1a into master Jan 21, 2019
@Dieterbe Dieterbe deleted the input-kafka-mdm branch January 21, 2019 18:10
@Dieterbe Dieterbe added this to the vnext milestone Feb 11, 2019
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.

kafka offset tracking should run in separate goroutine to consumer
3 participants