mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-10-20 02:42:29 +00:00
feat(cli): simplify packed metric exports with a boolean flag (#9531)
* feat(cli): select packed metric export with the layout option Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test(cli): inspect the packed capability error cause Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test(cli): follow the standard capability error source Signed-off-by: jeremyhi <fengjiachun@gmail.com> * refactor(cli): clarify the metric export gate helper name Signed-off-by: jeremyhi <fengjiachun@gmail.com> * feat(cli): use a boolean flag for packed metric exports Signed-off-by: jeremyhi <fengjiachun@gmail.com> --------- Signed-off-by: jeremyhi <fengjiachun@gmail.com>
This commit is contained in:
@@ -38,10 +38,11 @@
|
||||
//! --end-time 2025-01-31T23:59:59Z
|
||||
//! ```
|
||||
//!
|
||||
//! `--experimental-metric-export` enables shared Metric physical scans for Parquet.
|
||||
//! `--pack-metric-data` enables shared Metric physical scans for Parquet
|
||||
//! and writes version-2 packed snapshots.
|
||||
//! Enable `experimental_metric_export = true` on the frontend or standalone server
|
||||
//! and use an endpoint whose frontends all support and enable this option.
|
||||
//! Add `--metric-data-layout packed` to write version-2 packed snapshots.
|
||||
//! Resume reads the layout from the existing manifest.
|
||||
//! This requires the server packed-export capability; schema-only exports remain version 1.
|
||||
//! The command checks the server capability before modifying the snapshot.
|
||||
//! Each snapshot path belongs to one export task. Resume requires stable source
|
||||
|
||||
@@ -292,14 +292,11 @@ pub struct ExportCreateCommand {
|
||||
#[clap(long, value_enum, default_value = "parquet")]
|
||||
format: DataFormat,
|
||||
|
||||
/// Use shared Metric physical scans (Parquet only). Resume requires that the previous
|
||||
/// export and storage writes have ended; an HTTP timeout does not establish this.
|
||||
/// Use shared Metric physical scans and packed objects (Parquet only).
|
||||
/// Resume uses the manifest layout and requires that previous export and storage
|
||||
/// writes have ended; an HTTP timeout does not establish this.
|
||||
#[clap(long)]
|
||||
experimental_metric_export: bool,
|
||||
|
||||
/// Store independent Metric Parquet streams in shared packed objects.
|
||||
#[clap(long, value_parser = ["packed"], requires = "experimental_metric_export")]
|
||||
metric_data_layout: Option<String>,
|
||||
pack_metric_data: bool,
|
||||
|
||||
/// Delete existing snapshot and recreate.
|
||||
#[clap(long)]
|
||||
@@ -350,10 +347,7 @@ impl ExportCreateCommand {
|
||||
|
||||
let time_range = TimeRange::parse(self.start_time.as_deref(), self.end_time.as_deref())
|
||||
.map_err(BoxedError::new)?;
|
||||
if self.metric_data_layout.is_some()
|
||||
&& time_range.is_bounded()
|
||||
&& time_range.start == time_range.end
|
||||
{
|
||||
if self.pack_metric_data && time_range.is_bounded() && time_range.start == time_range.end {
|
||||
return crate::error::InvalidArgumentsSnafu {
|
||||
msg: "Packed export requires --start-time to be earlier than --end-time",
|
||||
}
|
||||
@@ -413,40 +407,18 @@ impl ExportCreateCommand {
|
||||
);
|
||||
|
||||
// The filesystem storage constructor can create the snapshot root.
|
||||
if self.experimental_metric_export {
|
||||
if self.format != DataFormat::Parquet {
|
||||
return crate::data::export_v2::error::MetricExportUnavailableSnafu
|
||||
.fail()
|
||||
.map_err(BoxedError::new);
|
||||
}
|
||||
let capability = database_client
|
||||
.sql_response(
|
||||
"SHOW VARIABLES experimental_metric_export",
|
||||
DEFAULT_SCHEMA_NAME,
|
||||
)
|
||||
if self.pack_metric_data {
|
||||
require_metric_export_enabled(&database_client, self.format)
|
||||
.await
|
||||
.context(DatabaseSnafu)
|
||||
.map_err(BoxedError::new)?;
|
||||
let expected_schema = OutputSchema::new(vec![ColumnSchema::new(
|
||||
"EXPERIMENTAL_METRIC_EXPORT".to_string(),
|
||||
"String".to_string(),
|
||||
)]);
|
||||
if !matches!(capability.output(), [GreptimeQueryOutput::Records(records)]
|
||||
if records.schema() == &expected_schema
|
||||
&& records.rows() == &vec![vec![Value::String("true".to_string())]])
|
||||
{
|
||||
return crate::data::export_v2::error::MetricExportUnavailableSnafu
|
||||
.fail()
|
||||
.map_err(BoxedError::new);
|
||||
if !self.schema_only {
|
||||
database_client
|
||||
.require_packed_export()
|
||||
.await
|
||||
.context(DatabaseSnafu)
|
||||
.map_err(BoxedError::new)?;
|
||||
}
|
||||
}
|
||||
if self.metric_data_layout.is_some() && !self.schema_only {
|
||||
database_client
|
||||
.require_packed_export()
|
||||
.await
|
||||
.context(DatabaseSnafu)
|
||||
.map_err(BoxedError::new)?;
|
||||
}
|
||||
let storage = OpenDalStorage::from_uri(&self.to, &self.storage).map_err(BoxedError::new)?;
|
||||
|
||||
Ok(Box::new(ExportCreate {
|
||||
@@ -455,8 +427,7 @@ impl ExportCreateCommand {
|
||||
schemas,
|
||||
schema_only: self.schema_only,
|
||||
format: self.format,
|
||||
experimental_metric_export: self.experimental_metric_export,
|
||||
packed: self.metric_data_layout.is_some(),
|
||||
packed: self.pack_metric_data,
|
||||
force: self.force,
|
||||
time_range,
|
||||
chunk_time_window: self.chunk_time_window,
|
||||
@@ -484,7 +455,6 @@ struct ExportConfig {
|
||||
schemas: Option<Vec<String>>,
|
||||
schema_only: bool,
|
||||
format: DataFormat,
|
||||
experimental_metric_export: bool,
|
||||
packed: bool,
|
||||
force: bool,
|
||||
time_range: TimeRange,
|
||||
@@ -538,13 +508,11 @@ impl ExportCreate {
|
||||
|
||||
validate_resume_config(&manifest, &self.config)?;
|
||||
if manifest.is_packed() {
|
||||
require_metric_export_enabled(&self.database_client, manifest.format).await?;
|
||||
self.database_client
|
||||
.require_packed_export()
|
||||
.await
|
||||
.context(DatabaseSnafu)?;
|
||||
if !self.config.experimental_metric_export {
|
||||
return crate::data::export_v2::error::MetricExportUnavailableSnafu.fail();
|
||||
}
|
||||
}
|
||||
|
||||
info!(
|
||||
@@ -573,7 +541,6 @@ impl ExportCreate {
|
||||
storage_config: &self.config.storage_config,
|
||||
parallelism: self.config.parallelism,
|
||||
chunk_parallelism: self.config.chunk_parallelism,
|
||||
experimental_metric_export: self.config.experimental_metric_export,
|
||||
resume: true,
|
||||
},
|
||||
progress.as_ref(),
|
||||
@@ -608,7 +575,7 @@ impl ExportCreate {
|
||||
manifest.version = 2;
|
||||
manifest.data_layout = Some(common_datasource::packed_snapshot::PACKED_LAYOUT.into());
|
||||
}
|
||||
if self.config.experimental_metric_export {
|
||||
if manifest.is_packed() {
|
||||
for chunk in &manifest.chunks {
|
||||
self.storage
|
||||
.prepare_export_chunk(&schema_names, chunk.id, false, manifest.is_packed())
|
||||
@@ -651,7 +618,6 @@ impl ExportCreate {
|
||||
storage_config: &self.config.storage_config,
|
||||
parallelism: self.config.parallelism,
|
||||
chunk_parallelism: self.config.chunk_parallelism,
|
||||
experimental_metric_export: self.config.experimental_metric_export,
|
||||
resume: false,
|
||||
},
|
||||
progress.as_ref(),
|
||||
@@ -761,10 +727,34 @@ impl ExportCreate {
|
||||
}
|
||||
}
|
||||
|
||||
async fn require_metric_export_enabled(client: &DatabaseClient, format: DataFormat) -> Result<()> {
|
||||
if format != DataFormat::Parquet {
|
||||
return crate::data::export_v2::error::MetricExportUnavailableSnafu.fail();
|
||||
}
|
||||
let capability = client
|
||||
.sql_response(
|
||||
"SHOW VARIABLES experimental_metric_export",
|
||||
DEFAULT_SCHEMA_NAME,
|
||||
)
|
||||
.await
|
||||
.context(DatabaseSnafu)?;
|
||||
let expected_schema = OutputSchema::new(vec![ColumnSchema::new(
|
||||
"EXPERIMENTAL_METRIC_EXPORT".to_string(),
|
||||
"String".to_string(),
|
||||
)]);
|
||||
if !matches!(capability.output(), [GreptimeQueryOutput::Records(records)]
|
||||
if records.schema() == &expected_schema
|
||||
&& records.rows() == &vec![vec![Value::String("true".to_string())]])
|
||||
{
|
||||
return crate::data::export_v2::error::MetricExportUnavailableSnafu.fail();
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_resume_config(manifest: &Manifest, config: &ExportConfig) -> Result<()> {
|
||||
if config.packed && !manifest.schema_only && !manifest.is_packed() {
|
||||
return ResumeConfigMismatchSnafu {
|
||||
field: "metric_data_layout",
|
||||
field: "data_layout",
|
||||
existing: "standalone".to_string(),
|
||||
requested: "packed".to_string(),
|
||||
}
|
||||
@@ -1683,6 +1673,44 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_pack_metric_data_arguments() {
|
||||
let directory = tempfile::tempdir().unwrap();
|
||||
let snapshot = directory.path().join("snapshot");
|
||||
let uri = url::Url::from_file_path(&snapshot).unwrap();
|
||||
let args = [
|
||||
"export-v2-create",
|
||||
"--addr",
|
||||
"127.0.0.1:1",
|
||||
"--to",
|
||||
uri.as_str(),
|
||||
];
|
||||
assert!(!ExportCreateCommand::parse_from(args).pack_metric_data);
|
||||
let packed =
|
||||
ExportCreateCommand::parse_from(args.into_iter().chain(["--pack-metric-data"]));
|
||||
assert!(packed.pack_metric_data);
|
||||
for removed in [
|
||||
&["--experimental-metric-export"][..],
|
||||
&["--metric-data-layout", "packed"][..],
|
||||
] {
|
||||
let error = ExportCreateCommand::try_parse_from(
|
||||
args.into_iter().chain(removed.iter().copied()),
|
||||
)
|
||||
.unwrap_err();
|
||||
assert_eq!(error.kind(), clap::error::ErrorKind::UnknownArgument);
|
||||
}
|
||||
for format in ["csv", "json"] {
|
||||
let command = ExportCreateCommand::parse_from(args.into_iter().chain([
|
||||
"--pack-metric-data",
|
||||
"--format",
|
||||
format,
|
||||
]));
|
||||
let error = command.build().await.err().unwrap();
|
||||
assert!(error.to_string().contains("Metric export requires Parquet"));
|
||||
assert!(!snapshot.exists());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_chunk_parallelism_parses_valid_value() {
|
||||
let cmd = ExportCreateCommand::parse_from([
|
||||
@@ -1755,7 +1783,6 @@ mod tests {
|
||||
schemas: None,
|
||||
schema_only: false,
|
||||
format: DataFormat::Parquet,
|
||||
experimental_metric_export: false,
|
||||
packed: false,
|
||||
force: false,
|
||||
time_range: TimeRange::unbounded(),
|
||||
@@ -1794,7 +1821,6 @@ mod tests {
|
||||
]),
|
||||
schema_only: false,
|
||||
format: DataFormat::Parquet,
|
||||
experimental_metric_export: false,
|
||||
packed: false,
|
||||
force: false,
|
||||
time_range: TimeRange::unbounded(),
|
||||
@@ -1812,7 +1838,7 @@ mod tests {
|
||||
validate_resume_config(&manifest, &config)
|
||||
.unwrap_err()
|
||||
.to_string()
|
||||
.contains("metric_data_layout")
|
||||
.contains("data_layout")
|
||||
);
|
||||
manifest.version = 2;
|
||||
manifest.data_layout = Some(common_datasource::packed_snapshot::PACKED_LAYOUT.into());
|
||||
@@ -1840,7 +1866,6 @@ mod tests {
|
||||
schemas: None,
|
||||
schema_only: false,
|
||||
format: DataFormat::Parquet,
|
||||
experimental_metric_export: false,
|
||||
packed: false,
|
||||
force: false,
|
||||
time_range,
|
||||
@@ -1875,7 +1900,6 @@ mod tests {
|
||||
schemas: None,
|
||||
schema_only: false,
|
||||
format: DataFormat::Csv,
|
||||
experimental_metric_export: false,
|
||||
packed: false,
|
||||
force: false,
|
||||
time_range: TimeRange::unbounded(),
|
||||
@@ -1912,7 +1936,6 @@ mod tests {
|
||||
schemas: None,
|
||||
schema_only: false,
|
||||
format: DataFormat::Parquet,
|
||||
experimental_metric_export: false,
|
||||
packed: false,
|
||||
force: false,
|
||||
time_range: TimeRange::new(Some(start), Some(start)),
|
||||
|
||||
@@ -39,7 +39,6 @@ struct ExportContext<'a> {
|
||||
schemas: Vec<String>,
|
||||
format: DataFormat,
|
||||
parallelism: usize,
|
||||
experimental_metric_export: bool,
|
||||
packed: bool,
|
||||
resume: bool,
|
||||
}
|
||||
@@ -49,7 +48,6 @@ pub struct ExportDataOptions<'a> {
|
||||
pub storage_config: &'a ObjectStoreConfig,
|
||||
pub parallelism: usize,
|
||||
pub chunk_parallelism: usize,
|
||||
pub experimental_metric_export: bool,
|
||||
pub resume: bool,
|
||||
}
|
||||
|
||||
@@ -74,7 +72,6 @@ pub async fn export_data(
|
||||
format: manifest.format,
|
||||
packed: manifest.is_packed(),
|
||||
parallelism: options.parallelism,
|
||||
experimental_metric_export: options.experimental_metric_export,
|
||||
resume: options.resume,
|
||||
};
|
||||
|
||||
@@ -278,7 +275,7 @@ async fn export_chunk(
|
||||
chunk_id: u32,
|
||||
time_range: TimeRange,
|
||||
) -> Result<Vec<String>> {
|
||||
if context.experimental_metric_export {
|
||||
if context.packed {
|
||||
context
|
||||
.storage
|
||||
.prepare_export_chunk(&context.schemas, chunk_id, context.resume, context.packed)
|
||||
@@ -290,7 +287,6 @@ async fn export_chunk(
|
||||
format: context.format,
|
||||
time_range,
|
||||
parallelism: context.parallelism,
|
||||
experimental_metric_export: context.experimental_metric_export,
|
||||
packed: context.packed,
|
||||
};
|
||||
|
||||
|
||||
@@ -30,7 +30,6 @@ pub(super) struct CopyOptions {
|
||||
pub(super) format: DataFormat,
|
||||
pub(super) time_range: TimeRange,
|
||||
pub(super) parallelism: usize,
|
||||
pub(crate) experimental_metric_export: bool,
|
||||
pub(super) packed: bool,
|
||||
}
|
||||
|
||||
@@ -218,10 +217,8 @@ pub(crate) async fn execute_copy_database_from(
|
||||
|
||||
fn build_with_options(options: &CopyOptions) -> String {
|
||||
let mut parts = vec![format!("FORMAT='{}'", options.format)];
|
||||
if options.experimental_metric_export {
|
||||
parts.push("experimental_metric_export='true'".to_string());
|
||||
}
|
||||
if options.packed {
|
||||
parts.push("experimental_metric_export='true'".to_string());
|
||||
parts.push("metric_data_layout='packed'".to_string());
|
||||
}
|
||||
if let Some(start) = options.time_range.start {
|
||||
@@ -430,6 +427,25 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::common::{PrefixedAzblobConnection, PrefixedGcsConnection, PrefixedOssConnection};
|
||||
|
||||
#[test]
|
||||
fn test_packed_copy_options() {
|
||||
let mut options = CopyOptions {
|
||||
format: DataFormat::Parquet,
|
||||
time_range: TimeRange::unbounded(),
|
||||
parallelism: 2,
|
||||
packed: false,
|
||||
};
|
||||
assert_eq!(
|
||||
build_with_options(&options),
|
||||
"FORMAT='parquet', PARALLELISM=2"
|
||||
);
|
||||
options.packed = true;
|
||||
assert_eq!(
|
||||
build_with_options(&options),
|
||||
"FORMAT='parquet', experimental_metric_export='true', metric_data_layout='packed', PARALLELISM=2"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_oss_connection_includes_endpoint() {
|
||||
let storage = ObjectStoreConfig {
|
||||
|
||||
Reference in New Issue
Block a user