Skip to content

lib/logstorage: fix time ordering in running_stats and total_stats#1591

Open
func25 wants to merge 3 commits into
masterfrom
fix-running-stats-time
Open

lib/logstorage: fix time ordering in running_stats and total_stats#1591
func25 wants to merge 3 commits into
masterfrom
fix-running-stats-time

Conversation

@func25

@func25 func25 commented Jul 10, 2026

Copy link
Copy Markdown
Member

running_stats and total_stats process rows in the wrong order as _time values were compared as text instead of timestamps.

Fixes them to use the actual timestamp value when available, and to parse string _time values before falling back to text ordering (backward compat). Also keep _time in updateNeededFields so ordering still works, this is documented.

Fixes #1581

@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.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread lib/logstorage/pipe_running_stats.go

@vadimalekseev vadimalekseev Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I understand it correctly, this PR contains both optimizations and the bug fix, I propose to only fix the bug to simplify the changes. See this git diff that also fixes the bug, but looks simplier:

diff --git a/lib/logstorage/pipe_running_stats.go b/lib/logstorage/pipe_running_stats.go
index 76c9a2009..9e6fa931c 100644
--- a/lib/logstorage/pipe_running_stats.go
+++ b/lib/logstorage/pipe_running_stats.go
@@ -271,7 +271,12 @@ func (psp *pipeRunningStatsProcessor) flush() error {
 	for _, key := range keys {
 		rows := m[key]
 		sort.Slice(rows, func(i, j int) bool {
-			return rows[i].timestamp < rows[j].timestamp
+			v1, ok1 := TryParseTimestampRFC3339Nano(rows[i].timestamp)
+			v2, ok2 := TryParseTimestampRFC3339Nano(rows[j].timestamp)
+			if !ok1 || !ok2 {
+				return rows[i].timestamp < rows[j].timestamp
+			}
+			return v1 < v2
 		})
 
 		if needStop(psp.stopCh) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah it is simpler, but I think the above would fix only the symptom and require the better fix anyway. Also this follows the timestamp-handling approach used by the sort pipe, so we could see it as keeping timestamp handling consistent across pipes rather than as a separate optimization.

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.

logsql: running_stats pipe may process logs in wrong time order

3 participants