Skip to content

Hottier fix #1645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
parmesant wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Hottier fix #1645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
86083cf
fix: hottier downloads
parmesant May 5, 2026
b26b90b
replace streaming with parallel downloads
parmesant May 5, 2026
92866c0
new task per range
parmesant May 5, 2026
881d6fe
expose new env vars
parmesant May 5, 2026
aec06d3
parallel file download per stream
parmesant May 5, 2026
2b786c7
concurrent writes instead of mutex
parmesant May 5, 2026
24b893f
crash safety by using .partial file
parmesant May 6, 2026
e734cf4
separate out historic and latest hottier tasks
parmesant May 6, 2026
60b8894
add logs
parmesant May 6, 2026
b5a348e
two loops instead of clockwerk
parmesant May 6, 2026
2b2fe61
per-stream hottier tasks
parmesant May 7, 2026
43e437c
fix: deepsource, coderabbit suggestions
parmesant May 8, 2026
b185b38
hottier deletion bug
parmesant May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,46 @@ pub struct Options {
)]
pub hot_tier_storage_path: Option<PathBuf>,

#[arg(
long = "hot-tier-download-chunk-size",
env = "P_HOT_TIER_DOWNLOAD_CHUNK_SIZE",
default_value = "8388608",
help = "Chunk size in bytes for parallel hot tier downloads (default 8 MiB)"
)]
pub hot_tier_download_chunk_size: u64,

#[arg(
long = "hot-tier-download-concurrency",
env = "P_HOT_TIER_DOWNLOAD_CONCURRENCY",
default_value = "16",
help = "Number of concurrent range requests per hot tier download"
)]
pub hot_tier_download_concurrency: usize,

#[arg(
long = "hot-tier-files-per-stream-concurrency",
env = "P_HOT_TIER_FILES_PER_STREAM_CONCURRENCY",
default_value = "4",
help = "Number of concurrent parquet file downloads per stream during hot tier sync"
)]
pub hot_tier_files_per_stream_concurrency: usize,

#[arg(
long = "hot-tier-latest-minutes",
env = "P_HOT_TIER_LATEST_MINUTES",
default_value = "10",
help = "Files whose timestamp is within the last N minutes are 'latest'; rest are 'historic'."
)]
pub hot_tier_latest_minutes: i64,

#[arg(
long = "hot-tier-historic-sync-minutes",
env = "P_HOT_TIER_HISTORIC_SYNC_MINUTES",
default_value = "5",
help = "Interval (minutes) at which the historic hot-tier sync runs."
)]
pub hot_tier_historic_sync_minutes: u32,
Comment on lines +341 to +355
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate the new hot-tier time knobs at parse time.

hot_tier_latest_minutes currently accepts negatives, and hot_tier_historic_sync_minutes accepts 0. With the new perpetual per-stream sync loops, that can turn the latest/historic split invalid or create a tight historic-sync loop. Please bound these in clap instead of relying on downstream code to recover.

Suggested fix 
     #[arg(
         long = "hot-tier-latest-minutes",
         env = "P_HOT_TIER_LATEST_MINUTES",
         default_value = "10",
+        value_parser = value_parser!(i64).range(1..),
         help = "Files whose timestamp is within the last N minutes are 'latest'; rest are 'historic'."
     )]
     pub hot_tier_latest_minutes: i64,

     #[arg(
         long = "hot-tier-historic-sync-minutes",
         env = "P_HOT_TIER_HISTORIC_SYNC_MINUTES",
         default_value = "5",
+        value_parser = value_parser!(u32).range(1..),
         help = "Interval (minutes) at which the historic hot-tier sync runs."
     )]
     pub hot_tier_historic_sync_minutes: u32,
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli.rs` around lines 341 - 355, Make these clap args enforce valid ranges
at parse time: for the field hot_tier_latest_minutes (i64) add a value_parser
that requires >= 1 (e.g. value_parser = clap::value_parser!(i64).range(1..)) so
negatives/zero are rejected, and for hot_tier_historic_sync_minutes (u32) add a
value_parser that requires >= 1 (e.g. value_parser =
clap::value_parser!(u32).range(1..)) so zero is rejected; update the #[arg(...)]
attributes for those two fields in the CLI struct accordingly.


//TODO: remove this when smart cache is implemented
#[arg(
long = "index-storage-path",
Expand Down
4 changes: 3 additions & 1 deletion src/handlers/http/logstream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,9 @@ pub async fn put_stream_hot_tier(
.metastore
.put_stream_json(&stream_metadata, &stream_name, &tenant_id)
.await?;

hot_tier_manager
.spawn_stream_tasks(stream_name.clone(), tenant_id.clone())
.await;
Ok((
format!("hot tier set for stream {stream_name}"),
StatusCode::OK,
Expand Down
Loading
Loading