Copilot commented on code in PR #980:
URL: https://github.com/apache/iceberg-cpp/pull/980#discussion_r4156622714
##########
src/iceberg/catalog/memory/in_memory_catalog.cc:
##########
@@ -605,13 +610,15 @@ Result<std::shared_ptr<Table>>
InMemoryCatalog::RegisterTable(
return InvalidArgument("file_io is not set for catalog {}", catalog_name_);
}
- ICEBERG_ASSIGN_OR_RAISE(auto metadata,
- TableMetadataUtil::Read(*file_io_,
metadata_file_location));
-
std::unique_lock lock(mutex_);
- if (!root_namespace_->NamespaceExists(identifier.ns)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto namespace_exists,
+ root_namespace_->NamespaceExists(identifier.ns));
+ if (!namespace_exists) {
return NoSuchNamespace("Table namespace does not exist: {}",
identifier.ns);
}
+
+ ICEBERG_ASSIGN_OR_RAISE(auto metadata,
+ TableMetadataUtil::Read(*file_io_,
metadata_file_location));
Review Comment:
This remote metadata read now runs while holding the catalog's exclusive
mutex, so a slow or blocked `FileIO` backend serializes every namespace and
table operation. `LoadTable` deliberately releases the mutex before
`TableMetadataUtil::Read` (`in_memory_catalog.cc:593-601`), and this read was
also outside the lock previously. Validate under the shared lock, release it
for I/O, then reacquire the exclusive lock and revalidate immediately before
registration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
