Skip to content

feat: Add time-window conversion via min/max TSDB offsets (day-rounded)#31

Open
TaranpreetSingh wants to merge 15 commits intothanos-io:mainfrom
TaranpreetSingh:timebased-sharding
Open

feat: Add time-window conversion via min/max TSDB offsets (day-rounded)#31
TaranpreetSingh wants to merge 15 commits intothanos-io:mainfrom
TaranpreetSingh:timebased-sharding

Conversation

@TaranpreetSingh
Copy link
Copy Markdown

@TaranpreetSingh TaranpreetSingh commented Nov 26, 2025

Introduce min/max TimeDate to support for conversion planning using convert.min-time and convert.max-time.

This change enables day-rounded time window selection for conversion via min/max time range. It improves operational control by focusing conversions on a targeted period, reducing unnecessary downloads, compute, and storage for out-of-window data. It supports backfill and selective reprocessing scenarios while preserving default behavior when duration are unset. Duration are interpreted as explicit date and rounded to day boundaries (min to 00:00:00 UTC; max to next day start, exclusive), ensuring consistent, predictable daily block generation.

Key changes

  • convert: use Planner.WithTimeWindow(min/max) in advanceConversion; existing per-day conversion loop unchanged.
  • TSDB discoverer: expose TimeOffsets() and option setters (TSDBMinTimeOffset/TSDBMaxTimeOffset) to pass CLI flags through.
  • Flags help text: clarify that min-time rounds to 00:00:00 UTC; max-time rounds to next day start (exclusive)
    Test result
    with flag used
Screenshot 2025-11-26 at 5 39 22 PM Without flag Screenshot 2025-11-26 at 5 41 27 PM

Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Comment thread internal/util/date.go Outdated
Comment thread convert/plan.go Outdated
Comment thread cmd/config.go Outdated
Comment thread locate/discover.go Outdated
Taranpreet Singh added 4 commits December 10, 2025 15:10
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Comment thread cmd/config.go Outdated
Comment thread convert/plan.go Outdated
Comment thread cmd/convert.go Outdated
}
log.Info("Converting next blocks", "sort_by", opts.conversion.sortLabels)
if err := advanceConversion(iterCtx, log, tsdbBkt, parquetBkt, tsdbDiscoverer, parquetDiscoverer, opts.conversion); err != nil {
if err := advanceConversion(iterCtx, log, tsdbBkt, parquetBkt, tsdbDiscoverer, parquetDiscoverer, opts.conversion, opts.conversion.minTimeOffset, opts.conversion.maxTimeOffset); err != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

they are already on conversion opts, we dont need to pass explicitely.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed, by removing them for the func parameter and calling them directly in plan

@MichaHoffmann
Copy link
Copy Markdown
Collaborator

Generally looks good to me, but can you address the nit above and maybe also clean up some of the comments and address the commented test issue?

Taranpreet Singh added 2 commits January 9, 2026 18:25
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Signed-off-by: Taranpreet Singh <tsingh@roku.com>
@TaranpreetSingh
Copy link
Copy Markdown
Author

Generally looks good to me, but can you address the nit above and maybe also clean up some of the comments and address the commented test issue?

When we added the time based sharding , it was not clear how should be passed the real time time range / set the time range for the test block in real time, as the plan use the time.now() cal for the time based sharding. This is been fixed now

Signed-off-by: Taranpreet Singh <TaranpreetSingh@users.noreply.github.com>
rebase with main changes

Signed-off-by: Taranpreet Singh <TaranpreetSingh@users.noreply.github.com>
rebases with the main

Signed-off-by: Taranpreet Singh <TaranpreetSingh@users.noreply.github.com>
Signed-off-by: Taranpreet Singh <TaranpreetSingh@users.noreply.github.com>
Copy link
Copy Markdown
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Why not just specify everything in days if everything is already rounded to a day? I'm trying to wrap my head around the problem this solves. Is the goal to run multiple converters for a different subset of inputs by day?

@TaranpreetSingh
Copy link
Copy Markdown
Author

Why not just specify everything in days if everything is already rounded to a day? I'm trying to wrap my head around the problem this solves. Is the goal to run multiple converters for a different subset of inputs by day?

Yes, as it can help in multiple ways, like in case of backfill.
On using the days instead of hours, that's a good point, we added this in format of hours to be align with the format of other config like grace period etc.

Signed-off-by: Taranpreet Singh <tsingh@roku.com>
Signed-off-by: Taranpreet Singh <TaranpreetSingh@users.noreply.github.com>
Comment thread convert/plan.go
winStartDay = util.NewDate(t.Year(), t.Month(), t.Day()).ToTime()
// Parse minDate and round to start of day (00:00:00 UTC, inclusive)
if minTimeDate != "" {
t, err := time.Parse("2006-01-02", minTimeDate)
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.

Same constant 2006-01-02 could be reused. Also, could we move validation one layer up - the caller of this function could check if the inputs are correct and just pass time.Time, no? If a non-empty string is passed then I would expect it to be checked and error out if it doesn't follow the format that is expected.

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.

3 participants