lib/vmselectapi: do not send empty label names for labelNames request (#4936)

* lib/vmselectapi: do not send empty label names for labelNames request
it breaks cluster communication, since vmselect incorrectly reads request buffer, leaving unread data on it
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4932

* typo fix

* wip

---------

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>
This commit is contained in:
Nikolay 2023-09-01 23:24:51 +02:00 committed by Aliaksandr Valialkin
parent f8105cebc9
commit e97484640a
No known key found for this signature in database
GPG key ID: A72BEC6CD3D0DED1
2 changed files with 9 additions and 1 deletions

View file

@ -25,6 +25,7 @@ The v1.87.x line will be supported for at least 12 months since [v1.87.0](https:
* BUGFIX: [build](https://docs.victoriametrics.com/): fix Docker builds for old Docker releases. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4907).
* BUGFIX: vmselect: correctly handle requests with `/select/multitenant` prefix. Such requests must be rejected since vmselect does not support multitenancy endpoint. Previously, such requests were causing panic. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4910).
* BUGFIX: [vminsert](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): properly check for [read-only state](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#readonly-mode) at `vmstorage`. Previously it wasn't properly checked, which could lead to increased resource usage and data ingestion slowdown when some of `vmstorage` nodes are in read-only mode. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4870).
* BUGFIX: [vmstorage](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): prevent from breaking `vmselect` -> `vmstorage` RPC communication when `vmstorage` returns an empty label name at `/api/v1/labels` request. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4932).
* BUGFIX: [vminsert](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): properly close broken vmstorage connection during [read-only state](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#readonly-mode) checks at `vmstorage`. Previously it wasn't properly closed, which prevents restoring `vmstorage` node from read-only mode. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4870).
* BUGFIX: do not allow starting VictoriaMetrics components with improperly set boolean command-line flags in the form `-boolFlagName value`, since this leads to silent incomplete flags' parsing. This form should be replaced with `-boolFlagName=value`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4845).
* BUGFIX: properly replace `:` chars in label names with `_` when `-usePromCompatibleNaming` command-line flag is passed to `vmagent`, `vminsert` or single-node VictoriaMetrics. This addresses [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3113#issuecomment-1275077071).

View file

@ -669,6 +669,10 @@ func (s *Server) processLabelNames(ctx *vmselectRequestCtx) error {
// Send labelNames to vmselect
for _, labelName := range labelNames {
if len(labelName) == 0 {
// Skip empty label names, since they may break RPC communication with vmselect
continue
}
if err := ctx.writeString(labelName); err != nil {
return fmt.Errorf("cannot write label name %q: %w", labelName, err)
}
@ -720,7 +724,7 @@ func (s *Server) processLabelValues(ctx *vmselectRequestCtx) error {
// Send labelValues to vmselect
for _, labelValue := range labelValues {
if len(labelValue) == 0 {
// Skip empty label values, since they have no sense for prometheus.
// Skip empty label values, since they may break RPC communication with vmselect
continue
}
if err := ctx.writeString(labelValue); err != nil {
@ -898,6 +902,9 @@ func (s *Server) processTenants(ctx *vmselectRequestCtx) error {
// Send tenants to vmselect
for _, tenant := range tenants {
if len(tenant) == 0 {
logger.Panicf("BUG: unexpected empty tenant name")
}
if err := ctx.writeString(tenant); err != nil {
return fmt.Errorf("cannot write tenant %q: %w", tenant, err)
}