I believe this should accept Option<Duration>, especially that the default is Some. Now it's impossible to set no timeout with SessionBuilder's API.

I decided to be consistent with other optional timeout options. See for example SessionBuilder::keepalive_interval() It accepts Duration, and sets the corresponding option to Some(duration) as well.

If user wants to set any of these to None, he can do so by direct access to the field (fields are public):

sb.config.hostname_resolution_timeout = None;

Hmm, I dislike the idea of SessionBuilder exposing only subset of possible configuration options using methods, with some being available only through fields.
WDYT @Lorak-mmk ?

We could expose another methods (disable_hostname_resolution_timeout, disable_keepalive) which would be a bit more descriptive.

I'd use impl ToSocketAddrs, as it's more concise and we get nothing with explicit type parameter here.

Wow, I'm suprised that such a trait exists

This means that the timeout is effectively twice the value provided in the config...
I'm not convinced it's correct. This must be at least noted in documentation.

Hmm... you are right. I wonder whether we should leave it as is, or rather recompute the timeout for the second try in case first failed.

    let deadline = hostname_resolution_timeout.map(|dur| tokio::time::Instant::now() + dur);
    let addrs = match lookup_host_with_timeout(hostname, hostname_resolution_timeout).await {
        Ok(addrs) => itertools::Either::Left(addrs),
        // Use a default port in case of error, but propagate the original error on failure
        Err(e) => {
            let new_timeout =
                deadline.map(|deadline| deadline.duration_since(tokio::time::Instant::now()));
            let addrs = lookup_host_with_timeout((hostname, 9042), new_timeout)
                .await
                .or(Err(e))?;
            itertools::Either::Right(addrs)
        }
    };

If the first attempt failed from reasons different that a timeout, then such recomputation makes perfect sense. However, if it failed due to a timeout (because the wrong port somehow caused a timeout - idk how likely this scenario is), then we won't be able to do the second attempt.

Any ideas @Lorak-mmk?

We could also execute resolve_hostname function with a given timeout (i.e. tokio::time::timeout(t, resolve_hostname)). It's much easier and cleaner than recomputing the timeout. However, in such case it's still true that if first attempt takes too long, we won't be able to do a second attempt.

When can it happen that the port is not specified and what is the expected behavior of lookup_host in this case?

If it always fails immediately with a specific error, then we can check this error and perform lookup with default port with the same timeout.

If it can timeout, or take some time in general, then I'd perform another lookup with the same timeout and document it.

From my investigation, it looks like port is not used in the actual DNS lookup (which makes sense).

What's the difference between String and (String, u16), then?
When String is provided, both tokio and std::net expect that it is of form "host:port". If it's not the case, the error is returned immidiately. Otherwise, host part is used in DNS lookup which may timeout.

When (String, u16) is provided, the DNS lookup is performed based on the string - port is ignored during lookup (same as above).

This means that:

• if hostname is of form "addr:port", then the first lookup timeouts iff the second lookup (with default port) timeouts
• if hostname is not of this form, then the first lookup will fail immidiately. We can try with default port.

In that case, I think that we can match the error returned from lookup_host_with_timeout If it's something other than DnsLookupError::Timeout(_), then we can try again with the same timeout.

Ok. Let's describe this behavior in the docs.