Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 69 additions & 2 deletions src/mito2/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ use store_api::region_request::{
AffectedRows, RegionCatchupRequest, RegionOpenRequest, RegionRequest,
};
use store_api::sst_entry::{ManifestSstEntry, PuffinIndexMetaEntry, StorageSstEntry};
use store_api::storage::{FileId, FileRefsManifest, RegionId, ScanRequest, SequenceNumber};
use store_api::storage::{
FileId, FileRefsManifest, ProjectionInput, RegionId, ScanRequest, SequenceNumber,
};
use tokio::sync::{Semaphore, oneshot};

use crate::access_layer::RegionFilePathFactory;
Expand Down Expand Up @@ -1011,10 +1013,75 @@ impl EngineInner {
.map(|r| r.find_committed_sequence())
}

// TODO(fys): remove it later.
// for example: `idx:path.to.field,idx2:path2`
fn hack_scan_req(req: &mut ScanRequest) {
let Ok(raw) = std::env::var("GREPTIME_PROJECTION_NESTED_PATHS") else {
return;
};

let mut projection = Vec::new();
let mut nested_paths = Vec::new();

for item in raw
.split(',')
.map(str::trim)
.filter(|item| !item.is_empty())
{
let Some((idx_str, path_str)) = item.split_once(':') else {
warn!(
"Ignoring invalid GREPTIME_PROJECTION_NESTED_PATHS item without ':' separator: {}",
item
);
continue;
};

let Ok(idx) = idx_str.trim().parse::<usize>() else {
warn!(
"Ignoring invalid GREPTIME_PROJECTION_NESTED_PATHS index: {}",
idx_str
);
continue;
};

let path = path_str
.trim()
.split('.')
.map(str::trim)
.filter(|segment| !segment.is_empty())
.map(ToString::to_string)
.collect::<Vec<_>>();

if path.is_empty() {
warn!(
"Ignoring empty nested path in GREPTIME_PROJECTION_NESTED_PATHS item: {}",
item
);
continue;
}

projection.push(idx);
nested_paths.push(path);
}

if projection.is_empty() {
return;
}

req.projection_input = Some(
ProjectionInput::new()
.with_projection(projection)
.with_nested_paths(nested_paths),
);

info!("projection_input hacked by env: {:?}", req.projection_input);
}

/// Handles the scan `request` and returns a [ScanRegion].
#[tracing::instrument(skip_all, fields(region_id = %region_id))]
fn scan_region(&self, region_id: RegionId, request: ScanRequest) -> Result<ScanRegion> {
fn scan_region(&self, region_id: RegionId, mut request: ScanRequest) -> Result<ScanRegion> {
let query_start = Instant::now();
Self::hack_scan_req(&mut request);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid overriding scan projection from global env hook

Calling hack_scan_req on every scan_region mutates request.projection_input for all scans whenever GREPTIME_PROJECTION_NESTED_PATHS is present, so one debug env setting can silently replace the planner-provided projection for unrelated queries. In that mode, the engine can return a different column set/schema than the query planned for, which can produce incorrect results or runtime schema-mismatch failures; this should be gated to explicit debug builds/paths instead of the default scan path.

Useful? React with 👍 / 👎.

// Reading a region doesn't need to go through the region worker thread.
let region = self.find_region(region_id)?;
let version = region.version();
Expand Down
2 changes: 2 additions & 0 deletions src/mito2/src/sst/parquet/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,8 @@ impl ParquetReaderBuilder {
let parquet_schema_desc = parquet_meta.file_metadata().schema_descr();
let parquet_read_cols = read_format.parquet_read_columns();
let leaf_indices = build_parquet_leaves_indices(parquet_schema_desc, &parquet_read_cols);
// TODO(fys): remove it later.
common_telemetry::info!("leaf_indices: {:?}", leaf_indices);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove info-level leaf index dump in hot read path

This adds an unconditional info! log for leaf_indices during parquet reader construction, which runs for normal scans and compactions; on wide schemas and many files this emits large per-file logs and adds avoidable CPU/I/O overhead in production. Because this is debug-only data, it should be removed or downgraded behind a debug/trace guard.

Useful? React with 👍 / 👎.

let projection_mask =
ProjectionMask::leaves(parquet_schema_desc, leaf_indices.iter().copied());
let selection = self
Expand Down
Loading