Could you suggest to code snippet how to properly initialize Azure cpp SDK for respect keep-alive server header?

[Slach]

Query/Question
I got some flaky errors inside my tests in clickhouse-server with azurite
details here
ClickHouse/ClickHouse#60447 and here Azure/Azurite#2443

clickhouse-server use aws-cpp-sdk to make requests to azurite
but after receive from azurite

Connection: keep-alive
Keep-Alive: timeout=5

and FIN TCP packet
aws-sdk/clickhouse-server doesn't send FIN ACK and trying to write to already closed connection
unfortunatelly in ClickHouse/ClickHouse@d7fb851
clickhouse team decided just retry on any transport error

Why is this not a Bug or a feature Request?

Could you suggest to code snippet how to properly initialize Azure cpp SDK for respect keep-alive server header?

[github-actions]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[LarryOsterman]

QQ: The Azure SDK supports two separate transports - one for WinHTTP, the other for libCURL. Which transport are you using?

WinHTTP should support the keep-alive header properly, but with a quick look at the code, the value of the "keep-alive" header does not appear to be respected, only that the presence of the header.

[Slach]

ClickHouse used libCURL

[Jinming-Hu]

How is this related to Azure SDK?
I think you should open issue either in https://github.com/aws/aws-sdk-cpp repo or https://github.com/Azure/Azurite repo.

[Slach]

@Jinming-Hu it definitely related to Azure SDK

ClickHouse uses Azure SDK, under non windows OS
Azure SDK use libCURL, may i ask you again code snippet

how to initialize Azure SDK properly, to respect server side header Keep-alive: timeout=5 which sente from azurite?

[Jinming-Hu]

We implement HTTP protocol on top of libcurl raw TCP connections. Looking through https://github.com/Azure/azure-sdk-for-cpp/blob/b4020493c4b0c2c1b5c81764be01b4ef0223bf4b/sdk/core/azure-core/src/http/curl/curl.cpp, I didn't find any handling of keep-alive header, I think it's just not supported.

[Slach]

didn't find any handling of keep-alive header, I think it's just not supported.
Look

azure-sdk-for-cpp/sdk/core/azure-core/src/http/curl/curl.cpp

Lines 930 to 985 in b402049

	if (non2xxAfter100ContinueWithNonzeroContentLength)
	{
	m_httpKeepAlive = false;
	}
	else
	{
	// HTTP <=1.0 is "close" by default. HTTP 1.1 is "keep-alive" by default.
	// The value can also be "keep-alive, close" (i.e. "both are fine"), in which case we are
	// preferring to treat it as keep-alive.
	// (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection)
	// Should it come to HTTP/2 and HTTP/3, they are "keep-alive", but any response from HTTP/2 or
	// /3 containing a "Connection" header should be considered malformed.
	// (HTTP/2: https://httpwg.org/specs/rfc9113.html#ConnectionSpecific
	// HTTP/3: https://httpwg.org/specs/rfc9114.html#rfc.section.4.2)
	//
	// HTTP/2+ are supposed to create persistent ("keep-alive") connections per host,
	// and close them by inactivity timeout. Given that we don't have such mechanism implemented
	// at this moment, we are closing all non-1.x connections immediately, which, in the worst case,
	// would only mean there's a perf hit, but the communication flow is expected to be correct.
	if (m_response->GetMajorVersion() == 1)
	{
	std::string connectionHeaderValue;
	{
	const Core::CaseInsensitiveMap& responseHeaders = m_response->GetHeaders();
	const auto connectionHeader = responseHeaders.find("Connection");
	if (connectionHeader != responseHeaders.cend())
	{
	connectionHeaderValue
	= Core::_internal::StringExtensions::ToLower(connectionHeader->second);
	}
	}
	const bool hasConnectionKeepAlive
	= connectionHeaderValue.find("keep-alive") != std::string::npos;
	if (m_response->GetMinorVersion() >= 1)
	{
	// HTTP/1.1+
	const bool hasConnectionClose = connectionHeaderValue.find("close") != std::string::npos;
	m_httpKeepAlive = (!hasConnectionClose || hasConnectionKeepAlive);
	}
	else
	{
	// HTTP/1.0
	m_httpKeepAlive = hasConnectionKeepAlive;
	}
	}
	else
	{
	// We don't expect HTTP/0.9, 2.0 or 3.0 in responses.
	// Barring rejecting as malformed, the safest thing to do here is to assume the connection is
	// not reusable.
	m_httpKeepAlive = false;
	}
	}

it should be supported

I just ask how to properly initialize SDK to handle standard HTTP behavior.

I believe, SDK shall not try to write to socket which already closed from server side and don't need to make useless packet send.

[Jinming-Hu]

@Slach the code snippet you quoted handles Connection: keep-alive. It doesn't handle keep-alive: xxxx

[Slach]

=) ok. got it, thanks for suggestion

But what about to add properly handle connection closing?
Let's convert this issue from question to bug-report?

When server sent Keep-Alive: timeout=5 and close connection after 5 second, then Azure CPP SDK will fire parasite traffic and fire execption when get TCP RST from server after try to write already closed connection

[LarryOsterman]

The Connection: Close header should be properly handled on all transports. What I believe is not correctly handled on curl is keep-alive: timeout. If the keep-alive header is present, the curl transport treats it the same as connection: keep-alive.

[gearama]

I suspect there is a similar issue on winhttp

[Slach]

@RickWinter thank you kindly for adding issue to milestone

i asked in libcurl repo suggestions
curl/curl#14487

[Slach]

maybe curl/curl#14487 (comment) could help

[LarryOsterman]

I think I was unclear: For performance reasons, the Azure SDK for C++'s libcurl transport does not use the libcurl implementation of HTTP, it only uses libcurl for connection management. All HTTP interactions are managed by the libcurl transport directly, as is all the connection pooling behavior.

I suspect this may be a bug in the Azure SDK for C++'s libCURL transport implementation which does not appear respect the parameters to the keep-alive HTTP header (I have not done any investigation on this issue however).