maintenance: make VictoriaMetrics flush recovery durable

[zqr10159]

Summary

• retain an unsuccessful VictoriaMetrics batch and retry it before draining later data
• recheck the queue after every immediate flush so writes added during an in-flight flush are scheduled
• when a periodic flush fails, schedule the retained batch on the one-second recovery path while keeping exactly one normal periodic chain
• reschedule the periodic chain from finally, and clear the pending guard when timer scheduling is rejected
• keep buffering bounded by applying producer backpressure during an outage instead of silently discarding data
• stop scheduling during shutdown, make one final synchronous drain, and reject later writes explicitly

Regression proof

The result-oriented contracts were run against the previous implementation first. They demonstrated that:

• only the first of two refilled batches was written
• a failed periodic write using the production-length 3600-second interval was attempted only once during a four-second observation window
• shutdown wrote none of the buffered metrics

The updated implementation proves that all refilled batches reach the HTTP boundary, the failed periodic batch is retried byte-for-byte on the one-second recovery path without creating a second periodic chain, and shutdown drains the queue once before rejecting later writes.

Validation

• ./mvnw -pl hertzbeat-warehouse -Dtest=VictoriaMetricsClusterDataStorageTest#retriesPeriodicFlushFailuresQuicklyWhenTheConfiguredIntervalIsLong test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false — passed with flushInterval=3600
• ./mvnw -pl hertzbeat-warehouse -Dtest=VictoriaMetricsClusterDataStorageTest test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false — 3 tests passed
• ./mvnw -pl hertzbeat-warehouse -am test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false — reactor passed; common-core 584, common-spring 102, warehouse 62 tests
• touched-file CJK scan and git diff --check — passed

Operational impact

No configuration migration is required. A periodic failure no longer waits for the full configured interval: retained data enters the bounded one-second retry path. During a sustained VictoriaMetrics outage, the queue applies backpressure to writers after one failed batch is retained. Shutdown performs a best-effort final flush and logs the retained item count if the destination still rejects it.

AI assistance: used for draft implementation and test iteration.
Human validation: executed the focused long-interval failure/recovery contract and the full warehouse reactor listed above.
Risk notes: recovery remains in-memory; a process crash cannot preserve buffered metrics, and prolonged destination outages can slow warehouse producers through deliberate backpressure.

[zqr10159]

Author remediation update:

Immediate flush completion now clears its guard in finally, rechecks the queue, and schedules the next batch when refill raced with the active flush. Failed HTTP batches remain queued and retry before later data, periodic scheduling is restored from finally while the timer is usable, rejected scheduling cannot leave the pending flag stuck, and shutdown performs a bounded best-effort drain with an observable retained count.

Refill-race, 503 retry and requeue, and shutdown-drain contracts passed; the full warehouse reactor also passed (62 tests). License and label checks are green; backend jobs are still running at the time of this update. Maintainer review remains required.

[zqr10159]

CI follow-up: backend build, Maven E2E, image E2E, license, and label checks have all completed successfully on the current head.

[zqr10159]

Addressed the long-interval periodic retry gap in the latest head (419e0e8658). A failed periodic flush now keeps the normal periodic chain and also schedules the retained batch through the bounded one-second immediate-recovery path. The regression uses flushInterval=3600; it failed on the previous implementation because only one HTTP attempt occurred in four seconds, and now proves a byte-identical second attempt. The focused class (3 tests) and the full warehouse reactor (584 common-core, 102 common-spring, 62 warehouse tests) pass.

[Duansg]

Thanks for this — the problems you're fixing are real, and I confirmed all three against master:

• MetricsFlushTask.run() drains a batch out of the queue and calls doSaveData(), which swallows failures internally, so a failed batch is silently lost;
• the overflow path calls doSaveData(contentList) — the whole list, once per item that failed to be offered — producing duplicate writes;
• the reschedule sits inside the try, so an exception escaping the flush kills the periodic chain permanently.

All worth fixing. But I think the new backpressure model is a blocking regression.

Unbounded backpressure freezes the persistence pipeline

sendVictoriaMetrics() now loops with no exit condition other than success or shutdown:

while (!offered) {
synchronized (metricsFlushLock) { offered = metricsBufferQueue.offer(content); }
if (offered) break;
triggerImmediateFlush();
Thread.sleep(MAX_WAIT_MS); // no bound, no attempt limit
}

saveData() runs on the single warehouse-persistent-data-storage thread (DataStorageDispatch.java:74-91), which also drives:

• calculateMonitorStatus() (line 86) — monitor up/down state
• pluginRunner.pluginExecute(PostCollectPlugin.class, ...) (line 88)
• realTimeDataWriter.saveData() (line 90) — real-time store backing the dashboard

If VictoriaMetrics is rejecting writes and the buffer fills, that thread blocks indefinitely and all four stop. metricsDataToStorageQueue is an unbounded LinkedBlockingQueue (InMemoryCommonDataQueue.java:58), so the backlog grows until the heap is exhausted.

To be precise about the blast radius: threshold alerting is not immediately affected — MetricsRealTimeAlertCalculator.java:132 consumes pollMetricsDataToAlerter(), a separate queue and thread. But once the storage queue exhausts the heap, that goes down with everything else.

The existing availability check does not save you here. checkVictoriaMetricsDatasourceAvailable() probes vmselect (VictoriaMetricsClusterDataStorage.java:147, vmClusterProps.select().url()), while this PR is about vminsert write failures. With vminsert down and vmselect healthy, serverAvailable stays true and execution falls straight into the loop above.

Trading bounded data loss for an unbounded stall is a bad trade for a monitoring system — a history-store outage should degrade history, not freeze status calculation and real-time writes.

Suggestion: either keep a bounded retry (N attempts / a total time budget) and then drop with a counter or metric, or move the VM write path onto its own executor so backpressure cannot propagate back into the shared consumer thread.

The same defects remain in the single-node storage

VictoriaMetricsDataStorage has essentially the same code and is untouched by this PR:

• sendVictoriaMetrics() at lines 572-612 still has the doSaveData(contentList) overflow path that re-sends the entire batch;
• MetricsFlushTask.run() at lines 627-651 still calls triggerIntervalFlushTimer() inside the try, so the chain can still die, and a drained batch is still lost on failure.

Since the single-node deployment is the more common one, fixing only the cluster variant leaves most users exposed. Worth either covering both here or extracting the shared flush logic.

Minor

Replacing log.error("... failed. {}", responseEntity.getBody()) with status-only logging removes the main diagnostic for write rejections (bad label, out of disk, retention policy). Keeping the body at DEBUG would preserve troubleshooting without the noise.