Skip to content

Commit f1abfd5

Browse files
committed
app/vlselect/logsql: consistently return zero hits on the selected time range from the /select/logsql/hits endpoint
Previously the endpoint could return responses with missing steps if there were no hits at these steps. This complicated handling of such responses at the client side. See #700
1 parent 01e49df commit f1abfd5

File tree

4 files changed

+60
-14
lines changed

4 files changed

+60
-14
lines changed

app/vlselect/logsql/hits_response.qtpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@
4949
"fields":{%s= k %},
5050
"timestamps":[
5151
{% if len(timestamps) > 0 %}
52-
{%q= timestamps[0] %}
52+
{%q= timestampToString(timestamps[0]) %}
5353
{% for _, ts := range timestamps[1:] %}
54-
,{%q= ts %}
54+
,{%q= timestampToString(ts) %}
5555
{% endfor %}
5656
{% endif %}
5757
],

app/vlselect/logsql/hits_response.qtpl.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/vlselect/logsql/logsql.go

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ func ProcessHitsRequest(ctx context.Context, w http.ResponseWriter, r *http.Requ
257257

258258
// Add a pipe, which calculates hits over time with the given step and offset for the given fields.
259259
ca.q.AddCountByTimePipe(int64(step), int64(offset), fields)
260+
start, end := ca.q.GetFilterTimeRange()
260261

261262
var mLock sync.Mutex
262263
m := make(map[string]*hitsSeries)
@@ -273,7 +274,10 @@ func ProcessHitsRequest(ctx context.Context, w http.ResponseWriter, r *http.Requ
273274

274275
bb := blockResultPool.Get()
275276
for i := 0; i < rowsCount; i++ {
276-
timestampStr := strings.Clone(timestampValues[i])
277+
timestampNsec, ok := logstorage.TryParseTimestampRFC3339Nano(timestampValues[i])
278+
if !ok {
279+
logger.Panicf("BUG: cannot parse timestamp=%q", timestampValues[i])
280+
}
277281
hitsStr := strings.Clone(hitsValues[i])
278282
hits, err := strconv.ParseUint(hitsStr, 10, 64)
279283
if err != nil {
@@ -286,11 +290,10 @@ func ProcessHitsRequest(ctx context.Context, w http.ResponseWriter, r *http.Requ
286290
mLock.Lock()
287291
hs, ok := m[string(bb.B)]
288292
if !ok {
289-
k := string(bb.B)
290293
hs = &hitsSeries{}
291-
m[k] = hs
294+
m[string(bb.B)] = hs
292295
}
293-
hs.timestamps = append(hs.timestamps, timestampStr)
296+
hs.timestamps = append(hs.timestamps, timestampNsec)
294297
hs.hits = append(hs.hits, hits)
295298
hs.hitsTotal += hits
296299
mLock.Unlock()
@@ -309,6 +312,7 @@ func ProcessHitsRequest(ctx context.Context, w http.ResponseWriter, r *http.Requ
309312
}
310313

311314
m = getTopHitsSeries(m, fieldsLimit)
315+
addMissingZeroHits(m, start, end, int64(step), int64(offset))
312316

313317
// Write response headers
314318
h := w.Header()
@@ -320,6 +324,47 @@ func ProcessHitsRequest(ctx context.Context, w http.ResponseWriter, r *http.Requ
320324
WriteHitsSeries(w, m)
321325
}
322326

327+
func addMissingZeroHits(m map[string]*hitsSeries, start, end, step, offset int64) {
328+
if start == math.MinInt64 {
329+
start = math.MaxInt64
330+
for _, hs := range m {
331+
start = min(start, slices.Min(hs.timestamps))
332+
}
333+
} else {
334+
start -= start%step - offset
335+
}
336+
337+
if end == math.MaxInt64 {
338+
end = math.MinInt64
339+
for _, hs := range m {
340+
end = max(end, slices.Max(hs.timestamps))
341+
}
342+
} else {
343+
end -= start%step - offset
344+
}
345+
346+
if start > end {
347+
// nothing to do
348+
return
349+
}
350+
351+
for _, hs := range m {
352+
ts := start
353+
for ts <= end {
354+
if !slices.Contains(hs.timestamps, ts) {
355+
hs.timestamps = append(hs.timestamps, ts)
356+
hs.hits = append(hs.hits, 0)
357+
}
358+
359+
if ts+step < ts {
360+
// stop on int64 overflow
361+
break
362+
}
363+
ts += step
364+
}
365+
}
366+
}
367+
323368
var blockResultPool bytesutil.ByteBufferPool
324369

325370
func getTopHitsSeries(m map[string]*hitsSeries, fieldsLimit int) map[string]*hitsSeries {
@@ -342,15 +387,15 @@ func getTopHitsSeries(m map[string]*hitsSeries, fieldsLimit int) map[string]*hit
342387
return a[i].hs.hitsTotal > a[j].hs.hitsTotal
343388
})
344389

345-
hitsOther := make(map[string]uint64)
390+
hitsOther := make(map[int64]uint64)
346391
for _, x := range a[fieldsLimit:] {
347-
for i, timestampStr := range x.hs.timestamps {
348-
hitsOther[timestampStr] += x.hs.hits[i]
392+
for i, timestamp := range x.hs.timestamps {
393+
hitsOther[timestamp] += x.hs.hits[i]
349394
}
350395
}
351396
var hsOther hitsSeries
352-
for timestampStr, hits := range hitsOther {
353-
hsOther.timestamps = append(hsOther.timestamps, timestampStr)
397+
for timestamp, hits := range hitsOther {
398+
hsOther.timestamps = append(hsOther.timestamps, timestamp)
354399
hsOther.hits = append(hsOther.hits, hits)
355400
hsOther.hitsTotal += hits
356401
}
@@ -366,7 +411,7 @@ func getTopHitsSeries(m map[string]*hitsSeries, fieldsLimit int) map[string]*hit
366411

367412
type hitsSeries struct {
368413
hitsTotal uint64
369-
timestamps []string
414+
timestamps []int64
370415
hits []uint64
371416
}
372417

docs/victorialogs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ according to the follosing docs:
2424
* FEATURE: [syslog data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/syslog/): add support for automatic parsing of [`@cee` messages](http://cee.mitre.org/language/1.0-beta1/clt.html#syslog). Thanks to @exherb for the pull request [#842](https://github.com/VictoriaMetrics/VictoriaLogs/pull/842).
2525
* FEATURE: [HTTP querying APIs](https://docs.victoriametrics.com/victorialogs/querying/#http-api): return `AccountID` and `ProjectID` response headers, which match the corresponding request headers when executing [queries for the particular tenant](https://docs.victoriametrics.com/victorialogs/#multitenancy). These response headers can be used by client such as [built-in web UI for VictoriaLogs](https://docs.victoriametrics.com/victorialogs/querying/#web-ui) for the proper displaying of the queried tenant. See [#656](https://github.com/VictoriaMetrics/VictoriaLogs/issues/656).
2626
* FETURE: [Elasticsearch data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/#elasticsearch-bulk-api): return the first parse error to the client, so the error could be quickly fixed. Previously the parse error was logged into VictoriaLogs internal logs and `success` response was returned to the client. This could complicate troubleshooting, since users need to have access to VictoriaLogs internal logs for the troubleshooting. See [#817](https://github.com/VictoriaMetrics/VictoriaLogs/issues/817) and [#60](https://github.com/VictoriaMetrics/VictoriaLogs/issues/60).
27+
* FEATURE: [hits API](https://docs.victoriametrics.com/victorialogs/querying/#querying-hits-stats): consistently return zero hits on the selected `[start .. end)` time range. Previously zero hits weren't returned, so this complicated processing of the hits response by clients. See [#700](https://github.com/VictoriaMetrics/VictoriaLogs/issues/700).
2728
* FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): add the help button with shortcuts reference and controls for charts and query input. See [#77](https://github.com/VictoriaMetrics/VictoriaLogs/issues/77).
2829
* FEATURE: [web UI](https://docs.victoriametrics.com/victorialogs/querying/#web-ui): auto-sync time picker with `_time` filter from the query (can be disabled). See [#558](https://github.com/VictoriaMetrics/VictoriaLogs/issues/558).
2930

0 commit comments

Comments
 (0)