Skip to content

lib/logstorage: fix uniq_values merge in cluster queries #1384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cuongleqq wants to merge 5 commits into
base: master
Choose a base branch
from
+37 −0
Open

lib/logstorage: fix uniq_values merge in cluster queries #1384

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
588985f
lib/logstorage: fix uniq_values merge in cluster queries
cuongleqq May 6, 2026
6202bb5
docs: make uniq_values cluster bug fix easier to understand
cuongleqq May 6, 2026
5476033
lib/logstorage: simplify uniq_values merge path
cuongleqq May 15, 2026
c9e2009
apptest: add test for uniq_values merge in cluster queries
cuongleqq May 16, 2026
2a4dc02
Merge branch 'master' into fix-uniq-values-empty-state-merge
cuongleqq May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/victorialogs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ according to the following docs:
* FEATURE: [alerts](https://github.com/VictoriaMetrics/VictoriaLogs/blob/master/deployment/docker/rules): add new alerting rules `PersistentQueueRunsOutOfSpaceIn12Hours` and `PersistentQueueRunsOutOfSpaceIn4Hours` for `vlagent` persistent queue capacity. These alerts help users to take proactive actions before `vlagent` starts dropping logs due to insufficient persistent queue space. See [#10193](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10193)
* FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): remove the `Date format` setting and always display timestamps with nanosecond precision. See [#1161](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1161).

* BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): fix panic when using [`uniq_values()`](https://docs.victoriametrics.com/victorialogs/logsql/#uniq_values-stats) in the [`stats` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#stats-pipe) in VictoriaLogs cluster when some `vlstorage` nodes return no values for the requested field while other `vlstorage` nodes return values for the same group. See [#1383](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1383).
* BUGFIX: [vlagent](https://docs.victoriametrics.com/victorialogs/vlagent/): hide sensitive values passed via `-remoteWrite.proxyURL` in `/metrics`, `/flags`, and startup logs. Previously these values could be exposed in plain text. See [#1320](https://github.com/VictoriaMetrics/VictoriaLogs/pull/1320).
* BUGFIX: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): sanitize markdown URLs in logs rendered with `markdown parsing` enabled, allowing only `http`, `https`, `mailto`, and `tel` schemes for active links and images. See [#1313](https://github.com/VictoriaMetrics/VictoriaLogs/pull/1313).
* BUGFIX: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): improve context view highlight visibility in dark theme. The selected log entry is now highlighted with a more visible blue tint instead of barely visible gray background. See [#1196](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1196).
Expand Down
6 changes: 6 additions & 0 deletions lib/logstorage/stats_uniq_values.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,18 @@ func (sup *statsUniqValuesProcessor) mergeState(_ *chunkedAllocator, sf statsFun
}

src := sfp.(*statsUniqValuesProcessor)
if len(src.m) == 0 {
Copy link
Copy Markdown
Contributor

@valyala valyala May 14, 2026

Choose a reason for hiding this comment

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

It is OK to skip this check - the loop below will work correctly and fast on empty map and on nil map.

Copy link
Copy Markdown
Contributor Author

@cuongleqq cuongleqq May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I kept the check because:

  1. It avoids allocating an empty map on a nil destination when the source is empty or nil.
  2. I am not entirely sure whether merging two nil maps should result in an empty map.

That said, the allocation is tiny and the case should not happen often, so I am fine with dropping the check if you prefer the simpler flow.

return
}
if len(src.m) > 100_000 {
// Postpone merging too big number of items in parallel
sup.ms = append(sup.ms, src.m)
return
}

if sup.m == nil {
sup.m = make(map[string]struct{}, len(src.m))
}
for k := range src.m {
if _, ok := sup.m[k]; !ok {
sup.m[k] = struct{}{}
Expand Down