Skip to content

Use curl multi interface - #60

Merged
connormanning merged 8 commits into
connormanning:masterfrom
abellgithub:multi
May 31, 2026
Merged

Use curl multi interface#60
connormanning merged 8 commits into
connormanning:masterfrom
abellgithub:multi

Conversation

@abellgithub

@abellgithub abellgithub commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

This replaces arbiter's use of the curl "easy" interface with the curl "multi" interface. This was recommended by the author of curl in order to reduce file descriptor use. Each easy "handle" would use at least three file descriptors to facilitate an http transfer. The multi interface uses a single descriptor for each transfer in addition to a couple of descriptors to facilitate asynchronous I/O. This should allow more transfers to occur without running out of file descriptors on system where they are a limited resource.

This change also does modifies the interface internally so that fetched data is moved rather than copied where possible.

@abellgithub

Copy link
Copy Markdown
Contributor Author

See PDAL/PDAL#4985

Comment thread arbiter/util/curl.cpp

const auto code(curl_easy_perform(m_curl));
curl_easy_getinfo(m_curl, CURLINFO_RESPONSE_CODE, &httpCode);
curl_easy_reset(m_curl);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we lost the curl_easy_reset - which would potentially allow settings from one request to persist through the next if not re-overridden.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. The settings are only made once when the Pool is made. Is this a problem?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying that without the curl_easy_reset in perform, if we call two different verbs that reuse the same handle, their non-identical options would not be reset from the first. So, if we do a PUT, which does:

curl_easy_setopt(m_curl, CURLOPT_WRITEFUNCTION, Response::getCb);

And then we do a GET later, then the WRITEFUNCTION won't be reset. Or CURLOPT_NOBODY for a HEAD request.

only made once when the Pool is made

Maybe I'm misinterpreting the flow here, because I am talking about the context of perform executing on a per-call basis - not a one-time init thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Yes, I think you're correct.

Comment thread arbiter/util/http.cpp
{
throw std::runtime_error("Cannot acquire from empty pool");
int msgCnt;
CURLMsg *m = curl_multi_info_read(m_multi, &msgCnt);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs for this function say that m->data.result holds the error code for the transfer which doesn't look checked here (although maybe this is handled outside this scope somehow) - is this the spot where we previously made up the error code 550 or whatever? What happens for a non-HTTP-status code response, e.g. timeout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the structure is generic. The result state for each transfer comes from the easy handle, not the multi handle and that's checked and set with curl_easy_getinfo a little farther down.

Comment thread arbiter/util/http.cpp
Comment thread arbiter/util/types.hpp
@connormanning
connormanning merged commit ac25c79 into connormanning:master May 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants