opensearch-project / opensearch-rs

OpenSearch Rust Client
Apache License 2.0
63 stars 34 forks source link

[BUG] Client creation takes 7-10ms #199

Closed mooreniemi closed 1 year ago

mooreniemi commented 1 year ago

What is the bug?

I am following the default tutorial documentation and measured the elapsed time in creating the client. I observe it taking 7-10ms, sometimes slightly more. When search requests (my query workload) are only 20ms - 50ms, and the default implementation of the connection pool is one connection (see https://github.com/opensearch-project/opensearch-rs/issues/197) which may encourage implementations of creating multiple clients, well, this overhead is significant.

How can one reproduce the bug?

Follow the default tutorial for creating a client and measure the elapsed time (now to elapsed below) to create the client.

    let now = Instant::now();
    let conn_pool = SingleNodeConnectionPool::new(url?);
    let mut headers = headers::HeaderMap::new();
    headers.insert(headers::ACCEPT_ENCODING, "gzip".parse().unwrap());
    let transport = TransportBuilder::new(conn_pool)
        .auth(aws_config.clone().try_into()?)
        .disable_proxy()
        .headers(headers)
        .service_name(service_name)
        .timeout(Duration::from_millis(500))
        .build()?;
    let client = OpenSearch::new(transport);
    let elapsed = now.elapsed();
    println!("creating client elapsed: {:.2?}", elapsed,);

I see, eg.:

creating client elapsed: 9.98ms

What is the expected behavior?

I would expect client initialization to be well below 1ms.

What is your host/environment?

AL2.

mooreniemi commented 1 year ago

A suspicion, it may be due to blocking call for DNS that uses getadderinfo:

https://github.com/hyperium/hyper-util/blob/1ed4c2ccdb23f576eb7024555f08b376b9d5c9eb/src/client/connect/dns.rs#L121

https://docs.rs/hyper/latest/src/hyper/client/connect/dns.rs.html#121

I've seen that take 10-13ms before in Tokio console. Not yet confirmed in this scenario.

Xtansia commented 1 year ago

@mooreniemi As I've just responded in #197, you should only be creating and re-using a single client instance, not per-request. In that context I don't believe 7-10ms is particularly bad. If there's obvious superfluous work being done wasting time, then happy to look into improvements, however very little happens in the construction other than building a reqwest::Client which I'd imagine is where most of the time goes. So may mostly be outside our control.

mooreniemi commented 1 year ago

Makes sense, and I am inclined to agree this is reqwest's fault. Closing as low priority.