Fix TemporalBuffer overflow for long-running timeseries#799
Merged
SimonHeybrock merged 4 commits intomainfrom Mar 19, 2026
Merged
Fix TemporalBuffer overflow for long-running timeseries#799SimonHeybrock merged 4 commits intomainfrom
SimonHeybrock merged 4 commits intomainfrom
Conversation
…city TemporalBuffer had a hardcoded 10,000-element capacity limit. When FullHistoryExtractor (required_timespan=inf) filled the buffer, trimming dropped nothing (cutoff = -infinity keeps everything), causing a permanent ValueError on every subsequent update. This made timeseries plots stop updating after ~2-3 hours in production. Fixes #798. Two changes: - When timespan-based trimming doesn't free enough space, fall back to ring-buffer behavior: drop the minimum number of oldest elements to make room for new data. - Replace the flat 10,000-element default with a 20 MB memory budget. Capacity is computed from actual bytes per element (including coordinate buffers), giving ~7 days of 0-D scalar data at 1/sec and proportionally less for larger data types.
Avoid O(capacity) copy on every update when buffer is full: - _make_room drops at least 10% of capacity instead of 1 element, so the expensive VariableBuffer.drop() runs once per ~10% fills. - _trim_to_timespan short-circuits for inf timespan, skipping the pointless cutoff computation that can never drop anything.
nvaytet
reviewed
Mar 18, 2026
| # Calculate max_capacity from memory limit | ||
| # Calculate max_capacity from memory limit, accounting for all buffers | ||
| # (data + time coord + optional start_time/end_time coords) | ||
| if 'time' in data.dims: |
Member
There was a problem hiding this comment.
You are dividing by n-time everywhere, I think you can reduce duplication by doing something like
bytes_per_element = data.data.values.nbytes
bytes_per_element += data.coords['time'].values.nbytes
if 'start_time' in data.coords:
bytes_per_element += data.coords['start_time'].values.nbytes
if 'end_time' in data.coords:
bytes_per_element += data.coords['end_time'].values.nbytes
if 'time' in data.dims:
bytes_per_element /= n_time?
| bytes_per_element = data.data.values.nbytes / data.sizes['time'] | ||
| n_time = data.sizes['time'] | ||
| bytes_per_element = data.data.values.nbytes / n_time | ||
| bytes_per_element += data.coords['time'].values.nbytes / n_time |
Member
There was a problem hiding this comment.
Is the time coord always guaranteed to be there?
Member
Author
There was a problem hiding this comment.
Yes, otherwise the value cannot be extracted as a timeseries.
| for t in range(1, capacity): | ||
| buffer.add(make_single_slice([float(t)] * 2, float(t))) | ||
| assert buffer._data_buffer.size == capacity | ||
|
|
Member
There was a problem hiding this comment.
Can we assert result.coords['time'].values[0] == 0.0 before adding one more, to make sure that this was the thing that was responsible for dropping the old?
- Refactor bytes-per-element calculation to eliminate duplication by accumulating total bytes first, then dividing by n_time at the end - Add comment explaining that 'time' coord is always guaranteed to be present - Add assertion in ring-buffer overflow test to verify buffer is full before triggering the overflow
nvaytet
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TemporalBufferhits its hardcoded 10,000-element capacity. WithFullHistoryExtractor(required_timespan=inf), timespan-based trimming drops nothing, causing a permanentValueErroron every subsequent update.Fixes #798.
Test plan
required_timespan=inf🤖 Generated with Claude Code