Skip to content

lib/logstorage: sort tenantIDs before returning to the client (and before searching) - #1575

Merged
func25 merged 6 commits into
masterfrom
sort-tenant-ids
Jul 6, 2026
Merged

lib/logstorage: sort tenantIDs before returning to the client (and before searching)#1575
func25 merged 6 commits into
masterfrom
sort-tenant-ids

Conversation

@func25

@func25 func25 commented Jul 4, 2026

Copy link
Copy Markdown
Member

Fixes silent data loss when searching over multiple tenants since tenantIDs are binary-searched during block selection but weren't guaranteed sorted (getTenantIDs returns them in random map order).

Sort them in getSearchOptions and getTenantIDs for deterministic output (to the frontend also)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread app/vlselect/logsql/logsql.go Outdated

@cuongleqq cuongleqq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As far as I understand, this PR fixes 2 things (just to clarify @func25's intent for anyone reading this PR):

  1. /select/tenant_ids rarely changes, but the order is random, so it comes back different on every request. That's annoying for any client that doesn't sort it itself (e.g. the Grafana datasource, or a jq command): the response looks changed when it isn't. So this part is a usability improvement for the API.
  2. Internally, the search code supports multiple tenants and assumes the tenant list is sorted (binary search in searchByTenantIDs), but nothing made sure it was. This is just a safety net for now because every external caller passes a single tenant, so it isn't fixing a real data loss today.

The code looks good to me so far.

@func25

func25 commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

This is just a safety net for now because every external caller passes a single tenant, so it isn't fixing a real data loss today.

FYI, it's for multitenant query. See #1576

@func25
func25 merged commit dd52b5e into master Jul 6, 2026
9 checks passed
@func25
func25 deleted the sort-tenant-ids branch July 6, 2026 00:12
valyala added a commit that referenced this pull request Jul 15, 2026
… handler

- Unify merging and sorting tenantIDs into lib/logstorage.MergeTenantIDs() function.
  This removes duplicate code and makes it more maintainable.

- Clarify docs for the GetTenantIDs() function, so it returns tenantIDs on the [start...end] time range.

- Rename indexdb.searchTenants() to indexdb.getTenantIDs() for the sake of consistency naming with other getTenantIDs() functions.

- Sort allTenantIDs list only once just after its' creation in the TestStorageRunQuery(). Previously it was sorted many times
  in parallel tests. This could result in data races.

- Make SortTenantIDs function private (i.e. rename it to sortTenantIDs), since it is no longer used outside the lib/logstorage package.

This is a follow-up for the commit 1892406 ( #417 )
and the commit dd52b5e ( #1575 )
@valyala

valyala commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

FYI, this pull request has been included in VictoriaLogs v1.52.0.

dadezzz pushed a commit to dadezzz/infra_docker-compose that referenced this pull request Jul 17, 2026
…g to v1.52.0 (#533)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker.io/victoriametrics/victoria-logs](https://github.com/VictoriaMetrics/VictoriaLogs) | minor | `v1.51.0` → `v1.52.0` |

---

### Release Notes

<details>
<summary>VictoriaMetrics/VictoriaLogs (docker.io/victoriametrics/victoria-logs)</summary>

### [`v1.52.0`](https://github.com/VictoriaMetrics/VictoriaLogs/releases/tag/v1.52.0)

[Compare Source](VictoriaMetrics/VictoriaLogs@v1.51.0...v1.52.0)

Released at 2026-07-16

**Update note 1:** the base Docker image has been changed from [Alpine](https://www.alpinelinux.org/) to [distroless](https://github.com/googlecontainertools/distroless) in order to reduce an attack surface (The `distroless` base image doesn't contain any executables contrary to the Alpine base image). For debugging VictoriaLogs containers in Kubernetes it is recommended to use [`kubectl debug`](https://kubernetes.io/docs/reference/kubectl/generated/kubectl_debug/).

**Update note 2:** VictoriaLogs no longer provides a Docker image for the `linux/386` platform because the `distroless` base image [doesn't support this platform](GoogleContainerTools/distroless#881). Executable files for `linux/386` platform are still published at [the VictoriaLogs releases page](https://github.com/VictoriaMetrics/VictoriaLogs/releases).

- SECURITY: upgrade Go builder from Go1.26.4 to Go1.26.5. See [the list of issues addressed in Go1.26.5](https://github.com/golang/go/issues?q=milestone%3AGo1.26.5%20label%3ACherryPickApproved).

- FEATURE: switch base Docker image from [Alpine](https://www.alpinelinux.org/) to [distroless](https://github.com/GoogleContainerTools/distroless/) for VictoriaLogs, `vlagent` and `vlogscli`. This reduces the image size and attack surface. See [#&#8203;1228](VictoriaMetrics/VictoriaLogs#1228).

- FEATURE: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): add [`json_array_concat` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#json_array_concat-pipe) for joining JSON array items stored in the given [log field](https://docs.victoriametrics.com/victorialogs/keyconcepts/#data-model) into a string with the given delimiter. See [#&#8203;712](VictoriaMetrics/VictoriaLogs#712).

- FEATURE: [data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/): adjust [`_stream` field](https://docs.victoriametrics.com/victorialogs/keyconcepts/#stream-fields) value according to the actual log fields. This simplifies importing of previously modified VictoriaLogs-exported data. See [#&#8203;1122](VictoriaMetrics/VictoriaLogs#1122).

- FEATURE: [vlagent](https://docs.victoriametrics.com/victorialogs/vlagent/): add `-remoteWrite.basicAuth.usernameFile` command-line flag for dynamically reloading basic auth username for the corresponding `-remoteWrite.url` from the given file. See [operator#2371](VictoriaMetrics/operator#2371).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): improve hits chart tooltip timestamp display by hiding excessive precision. See [#&#8203;1533](VictoriaMetrics/VictoriaLogs#1533).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): add Quick start examples to autocomplete suggestions and show recent query history items alongside field and value suggestions. See [#&#8203;1498](VictoriaMetrics/VictoriaLogs#1498).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): persist query history in browser local storage and show the last run time for history items. See [#&#8203;1498](VictoriaMetrics/VictoriaLogs#1498).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): improve hits chart interval selector by using curated interval options and choosing a more suitable default step for the selected time range. See [#&#8203;1511](VictoriaMetrics/VictoriaLogs#1511).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): sync grouping in raw logs panel with Hits chart. Raw Logs are no longer grouped by `_stream` by default, and both panels now use the same `Group by` setting. See [#&#8203;1534](VictoriaMetrics/VictoriaLogs#1534).

- FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): persist the selected auto-refresh interval in the URL. See [#&#8203;1310](VictoriaMetrics/VictoriaLogs#1310).

- 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 [#&#8203;1383](VictoriaMetrics/VictoriaLogs#1383).

- BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): properly select the top `N` log entries in the [`json_values`](https://docs.victoriametrics.com/victorialogs/logsql/#json_values-stats) stats function with `sort by (...) limit N` when more than `N` logs match per group. Previously an arbitrary subset of the matching logs could be returned instead of the top `N` ones. The returned entries were correctly sorted, but they could be the wrong entries. See [#&#8203;1311](VictoriaMetrics/VictoriaLogs#1311).

- BUGFIX: [syslog data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/syslog/): avoid stamping [RFC3164](https://datatracker.ietf.org/doc/html/rfc3164) messages received right after the new year with the previous year (which could drop them under a short `-retentionPeriod`), and compute the year in `-syslog.timezone` instead of the server local timezone. See [#&#8203;1556](VictoriaMetrics/VictoriaLogs#1556).

- BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): fix [`field_names` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#field_names-pipe) not respecting [`hidden_fields_filters`](https://docs.victoriametrics.com/victorialogs/querying/#hidden-fields). Fields listed in `hidden_fields_filters` still appeared in `field_names` output, leaking their existence to the caller. See [#&#8203;1574](VictoriaMetrics/VictoriaLogs#1574).

- BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): fix a `vlselect` crash in [cluster mode](https://docs.victoriametrics.com/victorialogs/cluster/) on some valid queries. This happened when a [`math` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#math-pipe) operand had the same name as a function (such as `abs`, `round` or `floor`), or when `from` was used as a separator in the [`split` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#split-pipe). See [#&#8203;1518](VictoriaMetrics/VictoriaLogs#1518).

- BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): allow [`filter` pipes](https://docs.victoriametrics.com/victorialogs/logsql/#filter-pipe) without the `filter` prefix when the filter starts with a non-word token or the `not` keyword, such as `... | !foo`, `... | {host="x"}`, `... | >5`, `... | =foo` or `... | not foo`. These were unintentionally rejected with `unexpected pipe` in [v1.51.0](https://github.com/VictoriaMetrics/VictoriaLogs/releases/tag/v1.51.0). A non-word token (and the `not` operator) cannot clash with a pipe name, so it is unambiguously a filter. See [#&#8203;1522](VictoriaMetrics/VictoriaLogs#1522).

- BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): reject `field*` wildcards in the comma-separated (no parens) form of [`uniq`](https://docs.victoriametrics.com/victorialogs/logsql/#uniq-pipe), [`top`](https://docs.victoriametrics.com/victorialogs/logsql/#top-pipe) and [`unroll`](https://docs.victoriametrics.com/victorialogs/logsql/#unroll-pipe) pipes, consistently with their parens form, which already rejects wildcards. Previously a query such as `uniq by foo*` was accepted but could not be parsed back from its string representation. See [#&#8203;1487](VictoriaMetrics/VictoriaLogs#1487).

- BUGFIX: [cluster version](https://docs.victoriametrics.com/victorialogs/cluster/): `vlstorage` now returns the error and stops processing an `/internal/select/*` request when the request cannot be parsed, instead of continuing to handle it and writing the response twice.

- BUGFIX: [cluster version](https://docs.victoriametrics.com/victorialogs/cluster/): bump the internal protocol version for [the delete endpoints](https://docs.victoriametrics.com/victorialogs/#how-to-delete-logs), which switched to multipart encoding in [v1.51.0](https://github.com/VictoriaMetrics/VictoriaLogs/releases/tag/v1.51.0) but kept the previous version. A version mismatch between `vlselect` and `vlstorage` for delete requests is now reported instead of silently mishandled. All cluster components must be upgraded together to use delete.

- BUGFIX: [cluster version](https://docs.victoriametrics.com/victorialogs/cluster/): avoid `cannot connect to storage node at ...: EOF` errors after `vlselect` or `vlinsert` was idle for more than 60 seconds. See [#&#8203;1440](VictoriaMetrics/VictoriaLogs#1440).

- BUGFIX: [vlselect](https://docs.victoriametrics.com/victorialogs/): respect `-search.maxQueueDuration` for queued requests. Previously, when the number of concurrent requests exceeded `-search.maxConcurrentRequests`, the extra requests could be queued for much longer than `-search.maxQueueDuration` (up to `-search.maxQueryDuration`) before being rejected, so a few slow queries could stall other requests for everyone. See [#&#8203;1554](VictoriaMetrics/VictoriaLogs#1554).

- BUGFIX: [vlselect](https://docs.victoriametrics.com/victorialogs/cluster/): properly register `-vmalert.proxyURL` as a secret flag. Previously, this flag was registered after logger initialization and printed in plaintext on application startup.

- BUGFIX: [vlselect](https://docs.victoriametrics.com/victorialogs/cluster/): return `502 Bad Gateway` HTTP response code for incoming queries when one of the `vlstorage` nodes runs a VictoriaLogs version with an incompatible internal API instead of `400 Bad Request`. This is consistent with the `502 Bad Gateway` response returned when a `vlstorage` node is unavailable, and it allows building a proper failover scheme in high-availability setups. See [these docs](https://docs.victoriametrics.com/victorialogs/cluster/#high-availability).

- BUGFIX: [multi-level cluster setup](https://docs.victoriametrics.com/victorialogs/cluster/#multi-level-cluster-setup): properly return `502 Bad Gateway` HTTP response code when a `vlselect` node queries other `vlselect` nodes and the underlying `vlstorage` is unavailable, as described at [high availability](https://docs.victoriametrics.com/victorialogs/cluster/#high-availability) docs. This allows configuring proper failover schemes to a healthy cluster.

- BUGFIX: [`/select/tenant_ids`](https://docs.victoriametrics.com/victorialogs/querying/#querying-tenants): return tenant ids in a deterministic (sorted) order. Previously the order was random across requests. See [#&#8203;1575](VictoriaMetrics/VictoriaLogs#1575).

- BUGFIX: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): fix hits chart tooltip layout to prevent long values from overflowing the viewport. See [#&#8203;1536](VictoriaMetrics/VictoriaLogs#1536).

- BUGFIX: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): fix filters sidebar opening on mobile. See [#&#8203;1537](VictoriaMetrics/VictoriaLogs#1537).

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNjAuMiIsInVwZGF0ZWRJblZlciI6IjQzLjI2MC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants