Skip to content
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.

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

Added a --v2 hidden in the ingest client. #4097

Merged
merged 1 commit into from
Nov 8, 2023
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
6 changes: 6 additions & 0 deletions quickwit/quickwit-cli/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,12 @@ pub fn build_index_command() -> Command {
.short('w')
.help("Wait for all documents to be commited and available for search before exiting")
.action(ArgAction::SetTrue),
// TODO remove me after Quickwit 0.7.
Arg::new("v2")
.long("v2")
.help("Ingest v2 (experimental! Do not use me.)")
.hide(true)
.action(ArgAction::SetTrue),
Arg::new("force")
.long("force")
.short('f')
Expand Down
11 changes: 11 additions & 0 deletions quickwit/quickwit-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ pub struct ClientArgs {
pub connect_timeout: Option<Timeout>,
pub timeout: Option<Timeout>,
pub commit_timeout: Option<Timeout>,
pub ingest_v2: bool,
}

impl Default for ClientArgs {
Expand All @@ -115,6 +116,7 @@ impl Default for ClientArgs {
connect_timeout: None,
timeout: None,
commit_timeout: None,
ingest_v2: false,
}
}
}
Expand All @@ -128,6 +130,9 @@ impl ClientArgs {
if let Some(timeout) = self.timeout {
builder = builder.timeout(timeout);
}
if self.ingest_v2 {
builder = builder.enable_ingest_v2();
}
builder.build()
}

Expand Down Expand Up @@ -180,6 +185,11 @@ impl ClientArgs {
} else {
None
};
let ingest_v2 = if process_ingest {
matches.get_flag("v2")
} else {
false
};
let commit_timeout = if process_ingest {
if let Some(duration) = matches.remove_one::<String>("commit-timeout") {
Some(parse_duration_or_none(&duration)?)
Expand All @@ -194,6 +204,7 @@ impl ClientArgs {
connect_timeout,
timeout,
commit_timeout,
ingest_v2,
})
}
}
Expand Down
30 changes: 29 additions & 1 deletion quickwit/quickwit-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,32 @@ mod tests {
Ok(())
}

#[test]
fn test_parse_ingest_v2_args() {
let app = build_cli().no_binary_name(true);
let matches = app
.try_get_matches_from(["index", "ingest", "--index", "wikipedia", "--v2"])
.unwrap();
let command = CliCommand::parse_cli_args(matches).unwrap();
assert!(matches!(
command,
CliCommand::Index(IndexCliCommand::Ingest(
IngestDocsArgs {
client_args,
index_id,
input_path_opt: None,
batch_size_limit_opt: None,
commit_type: CommitType::Auto,
})) if &index_id == "wikipedia"
&& client_args.timeout.is_none()
&& client_args.connect_timeout.is_none()
&& client_args.commit_timeout.is_none()
&& client_args.cluster_endpoint == Url::from_str("http://127.0.0.1:7280").unwrap()
&& client_args.ingest_v2

));
}

#[test]
fn test_parse_ingest_args() -> anyhow::Result<()> {
let app = build_cli().no_binary_name(true);
Expand All @@ -207,6 +233,7 @@ mod tests {
&& client_args.connect_timeout.is_none()
&& client_args.commit_timeout.is_none()
&& client_args.cluster_endpoint == Url::from_str("http://127.0.0.1:8000").unwrap()
&& !client_args.ingest_v2
));

let app = build_cli().no_binary_name(true);
Expand Down Expand Up @@ -234,8 +261,8 @@ mod tests {
&& client_args.timeout.is_none()
&& client_args.connect_timeout.is_none()
&& client_args.commit_timeout.is_none()
&& !client_args.ingest_v2
&& batch_size_limit == Byte::from_str("8MB").unwrap()

));

let app = build_cli().no_binary_name(true);
Expand Down Expand Up @@ -263,6 +290,7 @@ mod tests {
&& client_args.timeout.is_none()
&& client_args.connect_timeout.is_none()
&& client_args.commit_timeout.is_none()
&& !client_args.ingest_v2
&& batch_size_limit == Byte::from_str("4KB").unwrap()
));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ macro_rules! ingest_json {
};
}

pub async fn ingest_with_retry(
pub(crate) async fn ingest_with_retry(
client: &QuickwitClient,
index_id: &str,
ingest_source: IngestSource,
Expand Down
2 changes: 1 addition & 1 deletion quickwit/quickwit-integration-tests/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@

mod cluster_sandbox;

pub use cluster_sandbox::{build_node_configs, ingest_with_retry, ClusterSandbox};
pub(crate) use cluster_sandbox::{ingest_with_retry, ClusterSandbox};
48 changes: 25 additions & 23 deletions quickwit/quickwit-rest-client/src/rest_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use reqwest::header::{HeaderMap, HeaderValue, CONTENT_TYPE};
use reqwest::{Client, ClientBuilder, Method, StatusCode, Url};
use serde::Serialize;
use serde_json::json;
use tracing::warn;

use crate::error::Error;
use crate::models::{ApiResponse, IngestSource, Timeout};
Expand Down Expand Up @@ -119,6 +120,8 @@ pub struct QuickwitClientBuilder {
ingest_timeout: Timeout,
/// Timeout for the ingest operations that require waiting for commit.
commit_timeout: Timeout,
/// Experimental: if true, use the ingest v2 endpoint.
ingest_v2: bool,
}

impl QuickwitClientBuilder {
Expand All @@ -130,6 +133,7 @@ impl QuickwitClientBuilder {
search_timeout: DEFAULT_CLIENT_SEARCH_TIMEOUT,
ingest_timeout: DEFAULT_CLIENT_INGEST_TIMEOUT,
commit_timeout: DEFAULT_CLIENT_COMMIT_TIMEOUT,
ingest_v2: false,
}
}

Expand All @@ -143,6 +147,12 @@ impl QuickwitClientBuilder {
self
}

pub fn enable_ingest_v2(mut self) -> Self {
warn!("ingest v2 experimental feature enabled!");
self.ingest_v2 = true;
self
}

pub fn search_timeout(mut self, timeout: Timeout) -> Self {
self.search_timeout = timeout;
self
Expand All @@ -160,13 +170,14 @@ impl QuickwitClientBuilder {

pub fn build(self) -> QuickwitClient {
let transport = Transport::new(self.base_url, self.connect_timeout);
QuickwitClient::new(
QuickwitClient {
transport,
self.timeout,
self.search_timeout,
self.ingest_timeout,
self.commit_timeout,
)
timeout: self.timeout,
search_timeout: self.search_timeout,
ingest_timeout: self.ingest_timeout,
commit_timeout: self.commit_timeout,
ingest_v2: self.ingest_v2,
}
}
}

Expand All @@ -181,25 +192,12 @@ pub struct QuickwitClient {
ingest_timeout: Timeout,
/// Timeout for the ingest operations that require waiting for commit.
commit_timeout: Timeout,
// TODO remove me after Quickwit 0.7 release.
// If true, rely on ingest v2
ingest_v2: bool,
}

impl QuickwitClient {
fn new(
transport: Transport,
timeout: Timeout,
search_timeout: Timeout,
ingest_timeout: Timeout,
commit_timeout: Timeout,
) -> Self {
Self {
transport,
timeout,
search_timeout,
ingest_timeout,
commit_timeout,
}
}

pub async fn search(
&self,
index_id: &str,
Expand Down Expand Up @@ -258,7 +256,11 @@ impl QuickwitClient {
on_ingest_event: Option<&(dyn Fn(IngestEvent) + Sync)>,
last_block_commit: CommitType,
) -> Result<(), Error> {
let ingest_path = format!("{index_id}/ingest");
let ingest_path = if self.ingest_v2 {
format!("{index_id}/ingest-v2")
} else {
format!("{index_id}/ingest")
};
let batch_size_limit = batch_size_limit_opt.unwrap_or(INGEST_CONTENT_LENGTH_LIMIT);
let mut batch_reader = match ingest_source {
IngestSource::File(filepath) => {
Expand Down