Skip to content

Commit 901f215

Browse files
drop unnecessary signing mutex
1 parent f26927e commit 901f215

4 files changed

Lines changed: 14 additions & 18 deletions

File tree

src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include "iceberg/catalog/rest/auth/sigv4_auth_manager.h"
2121

2222
#include <cstdlib>
23-
#include <mutex>
2423
#include <sstream>
2524

2625
#include <aws/core/Aws.h>
@@ -154,12 +153,9 @@ Result<HTTPRequest> SigV4AuthSession::Authenticate(const HTTPRequest& request) {
154153
Aws::Utils::HashingUtils::Base64Encode(sha256));
155154
}
156155

157-
{
158-
std::lock_guard<std::mutex> lock(signing_mutex_);
159-
if (!signer_->SignRequest(*aws_request)) {
160-
return std::unexpected<Error>(
161-
Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"});
162-
}
156+
if (!signer_->SignRequest(*aws_request)) {
157+
return std::unexpected<Error>(Error{.kind = ErrorKind::kAuthenticationFailed,
158+
.message = "SigV4 signing failed"});
163159
}
164160

165161
HTTPRequest signed_request{.method = delegate_request.method,
@@ -269,10 +265,10 @@ std::string SigV4AuthManager::ResolveSigningRegion(
269265
return it->second;
270266
}
271267
if (const char* env = std::getenv("AWS_REGION")) {
272-
return std::string(env);
268+
return {env};
273269
}
274270
if (const char* env = std::getenv("AWS_DEFAULT_REGION")) {
275-
return std::string(env);
271+
return {env};
276272
}
277273
return "us-east-1";
278274
}

src/iceberg/catalog/rest/auth/sigv4_auth_manager.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#pragma once
2121

2222
#include <memory>
23-
#include <mutex>
2423
#include <string>
2524
#include <unordered_map>
2625

@@ -48,8 +47,8 @@ namespace iceberg::rest::auth {
4847
///
4948
/// See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv.html
5049
///
51-
/// Thread safety: Authenticate() is thread-safe; concurrent signing calls are
52-
/// serialized by an internal mutex.
50+
/// Thread safety: Authenticate() is thread-safe as long as the delegate
51+
/// session is.
5352
class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession {
5453
public:
5554
/// SHA-256 hash of empty string, used for requests with no body.
@@ -87,8 +86,6 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession {
8786
std::shared_ptr<Aws::Auth::AWSCredentialsProvider> credentials_provider_;
8887
std::unique_ptr<Aws::Client::AWSAuthV4Signer> signer_;
8988
std::unordered_map<std::string, std::string> effective_properties_;
90-
// AWSAuthV4Signer::SignRequest mutates shared signer state.
91-
mutable std::mutex signing_mutex_;
9289
};
9390

9491
/// \brief An AuthManager that produces SigV4AuthSession instances.

src/iceberg/catalog/rest/http_client.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ std::unordered_map<std::string, std::string> MergeHeaders(
8282
}
8383

8484
cpr::Header ToCprHeader(const auth::HTTPRequest& request) {
85-
return cpr::Header(request.headers.begin(), request.headers.end());
85+
return {request.headers.begin(), request.headers.end()};
8686
}
8787

8888
/// \brief Append URL-encoded query parameters to a URL, sorted by key.

src/iceberg/test/sigv4_auth_test.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ TEST_F(SigV4AuthTest, TableSessionInheritsProperties) {
209209
auto catalog_session = manager_result.value()->CatalogSession(client_, properties);
210210
ASSERT_THAT(catalog_session, IsOk());
211211

212-
iceberg::TableIdentifier table_id{iceberg::Namespace{{"ns1"}}, "table1"};
212+
iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"ns1"}},
213+
.name = "table1"};
213214
std::unordered_map<std::string, std::string> table_props;
214215
auto table_session = manager_result.value()->TableSession(table_id, table_props,
215216
catalog_session.value());
@@ -474,7 +475,8 @@ TEST_F(SigV4AuthTest, TableSessionOverridesProperties) {
474475
{AuthProperties::kSigV4SigningRegion, "ap-southeast-1"},
475476
};
476477

477-
iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"};
478+
iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"db1"}},
479+
.name = "table1"};
478480
auto table_session = manager_result.value()->TableSession(table_id, table_props,
479481
catalog_session.value());
480482
ASSERT_THAT(table_session, IsOk());
@@ -506,7 +508,8 @@ TEST_F(SigV4AuthTest, TableSessionInheritsContextualOverrides) {
506508
{{AuthProperties::kSigV4SigningRegion, "eu-west-1"}}, catalog_session.value());
507509
ASSERT_THAT(ctx_session, IsOk());
508510

509-
iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"};
511+
iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"db1"}},
512+
.name = "table1"};
510513
auto table_session = manager_result.value()->TableSession(table_id, /*properties=*/{},
511514
ctx_session.value());
512515
ASSERT_THAT(table_session, IsOk());

0 commit comments

Comments
 (0)