Skip to content

fix(rest): make RestCatalog safe to share across threads - #1092

Open
smaheshwar-pltr wants to merge 5 commits into
apache:mainfrom
smaheshwar-pltr:rest-catalog-thread-safety
Open

smaheshwar-pltr wants to merge 5 commits into
apache:mainfrom
smaheshwar-pltr:rest-catalog-thread-safety

Conversation

@smaheshwar-pltr

@smaheshwar-pltr smaheshwar-pltr commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1091.

Note: contains changes from #1090 which is a pre-requisite for this. See 7f9a593 for the only actual commit.

smaheshwar-pltr and others added 3 commits November 4, 2026 22:01
HttpClient sent every request through one cpr::ConnectionPool, and
libcurl does not support sharing a connection cache between threads.
Overlapping requests, for example from token refresh or the metrics
executor, could crash. Give each in-flight request its own pool and
reuse idle pools.

HttpResponse now copies the status, body and headers out of
cpr::Response, whose curl handle must be freed before its pool is
reused.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
AsCatalog() read and wrote the cached default catalog without a lock,
so concurrent calls raced. It was the only state RestCatalog changed
after Make(). Guard it with a mutex and document RestCatalog as
thread-safe.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#include "iceberg/catalog/rest/http_client.h"

#include <map>
#include <mutex>

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.

Note: these changes are from #1090 which is a pre-requisite for this. See 7f9a593 for the only actual commit. I'm opening this PR for review + CI.

@smaheshwar-pltr
smaheshwar-pltr marked this pull request as ready for review November 4, 2026 23:57
smaheshwar-pltr and others added 2 commits November 5, 2026 17:28
Head() lost its RequestUri() call in a rebase, so relative paths were
sent without the base URI. Restore it, and check that every request
method resolves relative paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Make() accepts a custom HTTP client factory, and every catalog call goes
through that client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Make RestCatalog safe to share across threads

1 participant