Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,7 @@ CheckOptions:
value: '_'
- key: readability-identifier-naming.ProtectedMemberSuffix
value: '_'
- key: modernize-use-scoped-lock.WarnOnSingleLocks
value: 'false'

HeaderFilterRegex: 'src/iceberg|example'
26 changes: 13 additions & 13 deletions src/iceberg/catalog/memory/in_memory_catalog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
#include "iceberg/catalog/memory/in_memory_catalog.h"

#include <algorithm>
#include <iterator> // IWYU pragma: keep
#include <iterator>
#include <mutex>

#include "iceberg/exception.h"
#include "iceberg/table.h"
#include "iceberg/table_metadata.h"
#include "iceberg/util/macros.h"
Expand Down Expand Up @@ -337,42 +337,42 @@ std::string_view InMemoryCatalog::name() const { return catalog_name_; }

Status InMemoryCatalog::CreateNamespace(
const Namespace& ns, const std::unordered_map<std::string, std::string>& properties) {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->CreateNamespace(ns, properties);
}

Result<std::unordered_map<std::string, std::string>>
InMemoryCatalog::GetNamespaceProperties(const Namespace& ns) const {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->GetProperties(ns);
}

Result<std::vector<Namespace>> InMemoryCatalog::ListNamespaces(
const Namespace& ns) const {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->ListNamespaces(ns);
}

Status InMemoryCatalog::DropNamespace(const Namespace& ns) {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->DropNamespace(ns);
}

Result<bool> InMemoryCatalog::NamespaceExists(const Namespace& ns) const {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->NamespaceExists(ns);
}

Status InMemoryCatalog::UpdateNamespaceProperties(
const Namespace& ns, const std::unordered_map<std::string, std::string>& updates,
const std::unordered_set<std::string>& removals) {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->UpdateNamespaceProperties(ns, updates, removals);
}

Result<std::vector<TableIdentifier>> InMemoryCatalog::ListTables(
const Namespace& ns) const {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
const auto& table_names = root_namespace_->ListTables(ns);
ICEBERG_RETURN_UNEXPECTED(table_names);
std::vector<TableIdentifier> table_idents;
Expand Down Expand Up @@ -405,12 +405,12 @@ Result<std::shared_ptr<Transaction>> InMemoryCatalog::StageCreateTable(
}

Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier) const {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
return root_namespace_->TableExists(identifier);
}

Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool purge) {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
// TODO(Guotao): Delete all metadata files if purge is true.
return root_namespace_->UnregisterTable(identifier);
}
Expand All @@ -428,7 +428,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(

Result<std::string> metadata_location;
{
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
ICEBERG_ASSIGN_OR_RAISE(metadata_location,
root_namespace_->GetTableMetadataLocation(identifier));
}
Expand All @@ -443,7 +443,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(

Result<std::shared_ptr<Table>> InMemoryCatalog::RegisterTable(
const TableIdentifier& identifier, const std::string& metadata_file_location) {
std::unique_lock lock(mutex_);
std::lock_guard guard(mutex_);
if (!root_namespace_->NamespaceExists(identifier.ns)) {
return NoSuchNamespace("table namespace does not exist.");
}
Expand Down
10 changes: 5 additions & 5 deletions src/iceberg/catalog/rest/http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ Result<HttpResponse> HttpClient::Get(
const ErrorHandler& error_handler) {
cpr::Response response;
{
std::scoped_lock<std::mutex> lock(session_mutex_);
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers, params);
response = session_->Get();
}
Expand All @@ -156,7 +156,7 @@ Result<HttpResponse> HttpClient::Post(
const ErrorHandler& error_handler) {
cpr::Response response;
{
std::scoped_lock<std::mutex> lock(session_mutex_);
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
session_->SetBody(cpr::Body{body});
response = session_->Post();
Expand All @@ -176,7 +176,7 @@ Result<HttpResponse> HttpClient::PostForm(
cpr::Response response;

{
std::scoped_lock<std::mutex> lock(session_mutex_);
std::lock_guard guard(session_mutex_);

// Override default Content-Type (application/json) with form-urlencoded
auto form_headers = headers;
Expand Down Expand Up @@ -204,7 +204,7 @@ Result<HttpResponse> HttpClient::Head(
const ErrorHandler& error_handler) {
cpr::Response response;
{
std::scoped_lock<std::mutex> lock(session_mutex_);
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
response = session_->Head();
}
Expand All @@ -220,7 +220,7 @@ Result<HttpResponse> HttpClient::Delete(
const ErrorHandler& error_handler) {
cpr::Response response;
{
std::scoped_lock<std::mutex> lock(session_mutex_);
std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
response = session_->Delete();
}
Expand Down
2 changes: 1 addition & 1 deletion src/iceberg/table_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector {
Result<std::unique_ptr<TableMetadata>> Build();

/// \brief Destructor
~TableMetadataBuilder();
~TableMetadataBuilder() override;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find a warning here that it misses override.


// Delete copy operations (use BuildFrom to create a new builder)
TableMetadataBuilder(const TableMetadataBuilder&) = delete;
Expand Down
Loading