From 8e2ff15203ea108ea598cdb9458120d70a9c08f5 Mon Sep 17 00:00:00 2001 From: Ruixiang Tan <819464715@qq.com> Date: Thu, 25 Jul 2024 14:55:12 +0800 Subject: [PATCH] refactor(vmstorage): Refactor the code to reduce the time complexity of `MustAddRows` and improve readability (#6629) ### Describe Your Changes The original logic is not only highly complex but also poorly readable, so it can be modified to increase readability and reduce time complexity. --------- Co-authored-by: Zhu Jiekun --- docs/CHANGELOG.md | 1 + lib/storage/partition.go | 5 +++++ lib/storage/table.go | 16 +++++++--------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a83431ece..fb9c7f292 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,6 +33,7 @@ See also [LTS releases](./LTS-releases.md). * SECURITY: upgrade base docker image (Alpine) from 3.20.1 to 3.20.2. See [alpine 3.20.2 release notes](https://alpinelinux.org/posts/Alpine-3.20.2-released.html). +* FEATURE: [VictoriaMetrics Single-Node](https://docs.victoriametrics.com/) and [VictoriaMetrics Cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): Refactor the code located in the `MustAddRows` function of `vmstorage` to improve performance and readability. * FEATURE: [vmauth](./vmauth.md): add `keep_original_host` option, which can be used for proxying the original `Host` header from client request to the backend. By default the backend host is used as `Host` header when proxying requests to the configured backends. See [these docs](./vmauth.md#host-http-header). * FEATURE: [vmauth](./vmauth.md) now returns HTTP 502 status code when all upstream backends are not available. Previously, it returned HTTP 503 status code. This change aligns vmauth behavior with other well-known reverse-proxies behavior. diff --git a/lib/storage/partition.go b/lib/storage/partition.go index ab70c3791..68a17a6ce 100644 --- a/lib/storage/partition.go +++ b/lib/storage/partition.go @@ -849,6 +849,11 @@ func (pt *partition) HasTimestamp(timestamp int64) bool { return timestamp >= pt.tr.MinTimestamp && timestamp <= pt.tr.MaxTimestamp } +// TimeRangeInPartition returns true if the pt contains the given time range. +func (pt *partition) TimeRangeInPartition(minTimestamp, MaxTimestamp int64) bool { + return minTimestamp >= pt.tr.MinTimestamp && MaxTimestamp <= pt.tr.MaxTimestamp +} + // GetParts appends parts snapshot to dst and returns it. // // The appended parts must be released with PutParts. diff --git a/lib/storage/table.go b/lib/storage/table.go index bbb48405d..4084c31f8 100644 --- a/lib/storage/table.go +++ b/lib/storage/table.go @@ -267,7 +267,12 @@ func (tb *table) MustAddRows(rows []rawRow) { if len(rows) == 0 { return } - + // Get the maximum and minimum timestamps in the rows + maxTsInRows, minTsInRows := rows[0].Timestamp, rows[0].Timestamp + for i := range rows { + maxTsInRows = max(maxTsInRows, rows[i].Timestamp) + minTsInRows = min(minTsInRows, rows[i].Timestamp) + } // Verify whether all the rows may be added to a single partition. ptwsX := getPartitionWrappers() defer putPartitionWrappers(ptwsX) @@ -275,14 +280,7 @@ func (tb *table) MustAddRows(rows []rawRow) { ptwsX.a = tb.GetPartitions(ptwsX.a[:0]) ptws := ptwsX.a for i, ptw := range ptws { - singlePt := true - for j := range rows { - if !ptw.pt.HasTimestamp(rows[j].Timestamp) { - singlePt = false - break - } - } - if !singlePt { + if !ptw.pt.TimeRangeInPartition(minTsInRows, maxTsInRows) { continue }