lib/persistentqueue: skip zero-length blocks as corrupt data in readBlock

Zero-length blocks indicate file corruption. Valid blocks should always
have non-zero length. Previously PR
https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6241 worked
around this at the client level by silently skipping such blocks. Move
the check into readBlock, consistent with how other corrupted headers
are handled there, and remove the now-redundant empty-block guards from
the client.

Follow-up on discussion
https://github.com/VictoriaMetrics/VictoriaMetrics/pull/10932#discussion_r3264924234

app/vmagent/remotewrite/client.go

@@ -311,11 +311,6 @@ func (c *client) runWorker() {
 		if !ok {
 			return
 		}
-		if len(block) == 0 {
-			// skip empty data blocks from sending
-			// see https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6241
-			continue
-		}
 		go func() {
 			startTime := time.Now()
 			ch <- c.sendBlock(block)
@@ -530,10 +525,6 @@ func (c *client) drainInMemoryQueue(stopCtx context.Context, block []byte) {
 			// In this case it is guaranteed that fq will be empty
 			return
 		}
-		if len(block) == 0 {
-			// skip empty data blocks from sending
-			continue
-		}
 
 		// at this stage c.stopCh should be closed
 		// so sendBlock function should not perform retries

docs/victoriametrics/Cluster-VictoriaMetrics.md

@@ -566,7 +566,7 @@ The following optional command-line flags related to mTLS are supported:
 - `-cluster.tlsCAFile` can be set at `vminsert`, `vmselect` and `vmstorage` for verifying peer certificates issued with custom [certificate authority](https://en.wikipedia.org/wiki/Certificate_authority). By default, system-wide certificate authority is used for peer certificate verification.
 - `-cluster.tlsCipherSuites` can be set to the list of supported TLS cipher suites at `vmstorage`. See [the list of supported TLS cipher suites](https://pkg.go.dev/crypto/tls#pkg-constants).
 
-When `vmselect` runs with `-clusternativeListenAddr` command-line option, then it can be configured with `-clusternative.tls*` options similar to `-cluster.tls*` for accepting `mTLS` connections from top-level `vmselect` nodes in [multi-level cluster setup](#multi-level-cluster-setup).
+When `vmselect` or `vminsert` runs with `-clusternativeListenAddr` command-line option, then it can be configured with `-clusternative.tls*` options similar to `-cluster.tls*` for accepting `mTLS` connections from top-level `vmselect` or `vminsert` nodes in [multi-level cluster setup](#multi-level-cluster-setup).
 
 See [these docs](https://gist.github.com/f41gh7/76ed8e5fb1ebb9737fe746bae9175ee6) on how to set up mTLS in VictoriaMetrics cluster.
 

docs/victoriametrics/changelog/CHANGELOG.md

@@ -28,6 +28,7 @@ See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-rel
 
 * FEATURE: all VictoriaMetrics components: improve logging for the `-memory.allowedBytes` flag to warn about excessively low value (less than 1MB). See issue [#10935](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10935).
 * FEATURE: [vmagent](https://docs.victoriametrics.com/victoriametrics/vmagent/) and [vmalert](https://docs.victoriametrics.com/victoriametrics/vmalert/): add `basicAuth.usernameFile` command-line flags for reading basic auth username from a file, similar to the existing `basicAuth.passwordFile`. The file is re-read every second. See [#9436](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9436). Thanks to @kimjune01 for the contribution.
+* FEATURE: `vminsert` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): add `clusternative.tls` `vminsert` configuration flags for [multi-level cluster setups](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/#multi-level-cluster-setup). See [#10958](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10958).
 * FEATURE: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/), `vminsert` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/) and [vmagent](https://docs.victoriametrics.com/victoriametrics/vmagent/): add `-opentelemetry.labelNameUnderscoreSanitization` command-line flag to control whether to enable prepending of `key` to labels starting with `_` when `-opentelemetry.usePrometheusNaming` is enabled. See [OpenTelemetry](https://docs.victoriametrics.com/victoriametrics/integrations/opentelemetry/) docs and [#9663](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9663). Thanks to @andriibeee for the contribution.
 * FEATURE: [vmui](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/#vmui): improve the [Top Queries](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/#top-queries) table UI. Duration columns now display human-readable values (e.g. `1.23s`) instead of raw seconds, memory column shows human-readable sizes (e.g. `1.23 MB`), instant queries are labeled as `instant` instead of empty string, and column headers now show tooltips with descriptions. See [#10790](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/10790).
 * FEATURE: [vmagent](https://docs.victoriametrics.com/victoriametrics/vmagent/): drain in-memory remote write queue on shutdown within the 5-second grace period before falling back to persisting blocks to disk. See [#9996](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9996)

lib/persistentqueue/persistentqueue.go

@@ -485,6 +485,14 @@ again:
 		}
 		goto again
 	}
+	// see https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6241
+	if blockLen == 0 {
+		logger.Errorf("skipping corrupted %q, since zero block size is read from it", q.readerPath)
+		if err := q.skipBrokenChunkFile(); err != nil {
+			return dst, err
+		}
+		goto again
+	}
 	if blockLen > q.maxBlockSize {
 		logger.Errorf("skipping corrupted %q, since too big block size is read from it: %d bytes; cannot exceed %d bytes", q.readerPath, blockLen, q.maxBlockSize)
 		if err := q.skipBrokenChunkFile(); err != nil {