Merge pull request #67896 from CurtizJ/fix-azure-create-container

Do not try to create azure container if not needed
This commit is contained in:
Alexey Milovidov 2024-08-06 22:56:19 +00:00 committed by GitHub
commit ec83156691
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 46 additions and 3 deletions

View File

@ -459,6 +459,7 @@ The server successfully detected this situation and will download merged part fr
M(AzureDeleteObjects, "Number of Azure blob storage API DeleteObject(s) calls.") \ M(AzureDeleteObjects, "Number of Azure blob storage API DeleteObject(s) calls.") \
M(AzureListObjects, "Number of Azure blob storage API ListObjects calls.") \ M(AzureListObjects, "Number of Azure blob storage API ListObjects calls.") \
M(AzureGetProperties, "Number of Azure blob storage API GetProperties calls.") \ M(AzureGetProperties, "Number of Azure blob storage API GetProperties calls.") \
M(AzureCreateContainer, "Number of Azure blob storage API CreateContainer calls.") \
\ \
M(DiskAzureGetObject, "Number of Disk Azure API GetObject calls.") \ M(DiskAzureGetObject, "Number of Disk Azure API GetObject calls.") \
M(DiskAzureUpload, "Number of Disk Azure blob storage API Upload calls") \ M(DiskAzureUpload, "Number of Disk Azure blob storage API Upload calls") \
@ -466,8 +467,9 @@ The server successfully detected this situation and will download merged part fr
M(DiskAzureCommitBlockList, "Number of Disk Azure blob storage API CommitBlockList calls") \ M(DiskAzureCommitBlockList, "Number of Disk Azure blob storage API CommitBlockList calls") \
M(DiskAzureCopyObject, "Number of Disk Azure blob storage API CopyObject calls") \ M(DiskAzureCopyObject, "Number of Disk Azure blob storage API CopyObject calls") \
M(DiskAzureListObjects, "Number of Disk Azure blob storage API ListObjects calls.") \ M(DiskAzureListObjects, "Number of Disk Azure blob storage API ListObjects calls.") \
M(DiskAzureDeleteObjects, "Number of Azure blob storage API DeleteObject(s) calls.") \ M(DiskAzureDeleteObjects, "Number of Disk Azure blob storage API DeleteObject(s) calls.") \
M(DiskAzureGetProperties, "Number of Disk Azure blob storage API GetProperties calls.") \ M(DiskAzureGetProperties, "Number of Disk Azure blob storage API GetProperties calls.") \
M(DiskAzureCreateContainer, "Number of Disk Azure blob storage API CreateContainer calls.") \
\ \
M(ReadBufferFromAzureMicroseconds, "Time spent on reading from Azure.") \ M(ReadBufferFromAzureMicroseconds, "Time spent on reading from Azure.") \
M(ReadBufferFromAzureInitMicroseconds, "Time spent initializing connection to Azure.") \ M(ReadBufferFromAzureInitMicroseconds, "Time spent initializing connection to Azure.") \

View File

@ -11,6 +11,14 @@
#include <Poco/Util/AbstractConfiguration.h> #include <Poco/Util/AbstractConfiguration.h>
#include <Interpreters/Context.h> #include <Interpreters/Context.h>
namespace ProfileEvents
{
extern const Event AzureGetProperties;
extern const Event DiskAzureGetProperties;
extern const Event AzureCreateContainer;
extern const Event DiskAzureCreateContainer;
}
namespace DB namespace DB
{ {
@ -214,20 +222,53 @@ void processURL(const String & url, const String & container_name, Endpoint & en
} }
} }
static bool containerExists(const ContainerClient & client)
{
ProfileEvents::increment(ProfileEvents::AzureGetProperties);
if (client.GetClickhouseOptions().IsClientForDisk)
ProfileEvents::increment(ProfileEvents::DiskAzureGetProperties);
try
{
client.GetProperties();
return true;
}
catch (const Azure::Storage::StorageException & e)
{
if (e.StatusCode == Azure::Core::Http::HttpStatusCode::NotFound)
return false;
throw;
}
}
std::unique_ptr<ContainerClient> getContainerClient(const ConnectionParams & params, bool readonly) std::unique_ptr<ContainerClient> getContainerClient(const ConnectionParams & params, bool readonly)
{ {
if (params.endpoint.container_already_exists.value_or(false) || readonly) if (params.endpoint.container_already_exists.value_or(false) || readonly)
{
return params.createForContainer(); return params.createForContainer();
}
if (!params.endpoint.container_already_exists.has_value())
{
auto container_client = params.createForContainer();
if (containerExists(*container_client))
return container_client;
}
try try
{ {
auto service_client = params.createForService(); auto service_client = params.createForService();
ProfileEvents::increment(ProfileEvents::AzureCreateContainer);
if (params.client_options.ClickhouseOptions.IsClientForDisk)
ProfileEvents::increment(ProfileEvents::DiskAzureCreateContainer);
return std::make_unique<ContainerClient>(service_client->CreateBlobContainer(params.endpoint.container_name).Value); return std::make_unique<ContainerClient>(service_client->CreateBlobContainer(params.endpoint.container_name).Value);
} }
catch (const Azure::Storage::StorageException & e) catch (const Azure::Storage::StorageException & e)
{ {
/// If container_already_exists is not set (in config), ignore already exists error. /// If container_already_exists is not set (in config), ignore already exists error. Conflict - The specified container already exists.
/// (Conflict - The specified container already exists) /// To avoid race with creation of container, handle this error despite that we have already checked the existence of container.
if (!params.endpoint.container_already_exists.has_value() && e.StatusCode == Azure::Core::Http::HttpStatusCode::Conflict) if (!params.endpoint.container_already_exists.has_value() && e.StatusCode == Azure::Core::Http::HttpStatusCode::Conflict)
return params.createForContainer(); return params.createForContainer();
throw; throw;