Skip to content

Commit

Permalink
Fix error when deploy with KS3 (#7485) (#7494)
Browse files Browse the repository at this point in the history
close #7484
  • Loading branch information
ti-chi-bot authored May 19, 2023
1 parent 27eb2ed commit 02bf222
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 11 deletions.
2 changes: 1 addition & 1 deletion contrib/aws
11 changes: 6 additions & 5 deletions dbms/src/Storages/S3/PocoHTTPClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,17 +281,18 @@ std::optional<String> PocoHTTPClient::makeRequestOnce(
const std::string reserved = "?#:;+@&=%"; /// Poco::URI::RESERVED_QUERY_PARAM without '/' plus percent sign.
Poco::URI::encode(target_uri.getPath(), reserved, path_and_query);

/// `target_uri.getPath()` could return an empty string, but a proper HTTP request must
/// always contain a non-empty URI in its first line (e.g. "POST / HTTP/1.1" or "POST /?list-type=2 HTTP/1.1").
if (path_and_query.empty())
path_and_query = "/";

/// Append the query param to URI
if (!query.empty())
{
path_and_query += '?';
path_and_query += query;
}

/// `target_uri.getPath()` could return an empty string, but a proper HTTP request must
/// always contain a non-empty URI in its first line (e.g. "POST / HTTP/1.1").
if (path_and_query.empty())
path_and_query = "/";

poco_request.setURI(path_and_query);

switch (request.GetMethod())
Expand Down
13 changes: 10 additions & 3 deletions dbms/src/Storages/S3/S3Common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -991,10 +991,17 @@ void deleteObject(const TiFlashS3Client & client, const String & key)
auto o = client.DeleteObject(req);
if (!o.IsSuccess())
{
throw fromS3Error(o.GetError(), "S3 DeleteObject failed, bucket={} root={} key={}", client.bucket(), client.root(), key);
const auto & e = o.GetError();
if (e.GetErrorType() != Aws::S3::S3Errors::NO_SUCH_KEY)
{
throw fromS3Error(o.GetError(), "S3 DeleteObject failed, bucket={} root={} key={}", client.bucket(), client.root(), key);
}
}
else
{
const auto & res = o.GetResult();
UNUSED(res);
}
const auto & res = o.GetResult();
UNUSED(res);
GET_METRIC(tiflash_storage_s3_request_seconds, type_delete_object).Observe(sw.elapsedSeconds());
}

Expand Down
4 changes: 2 additions & 2 deletions dbms/src/TestUtils/TiFlashTestEnv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ bool TiFlashTestEnv::createBucketIfNotExist(::DB::S3::TiFlashS3Client & s3_clien
{
LOG_DEBUG(s3_client.log, "Created bucket {}", s3_client.bucket());
}
else if (outcome.GetError().GetExceptionName() == "BucketAlreadyOwnedByYou")
else if (outcome.GetError().GetExceptionName() == "BucketAlreadyOwnedByYou" || outcome.GetError().GetExceptionName() == "BucketAlreadyExists")
{
LOG_DEBUG(s3_client.log, "Bucket {} already exist", s3_client.bucket());
}
Expand All @@ -259,7 +259,7 @@ bool TiFlashTestEnv::createBucketIfNotExist(::DB::S3::TiFlashS3Client & s3_clien
const auto & err = outcome.GetError();
LOG_ERROR(s3_client.log, "CreateBucket: {}:{}", err.GetExceptionName(), err.GetMessage());
}
return outcome.IsSuccess() || outcome.GetError().GetExceptionName() == "BucketAlreadyOwnedByYou";
return outcome.IsSuccess() || outcome.GetError().GetExceptionName() == "BucketAlreadyOwnedByYou" || outcome.GetError().GetExceptionName() == "BucketAlreadyExists";
}

void TiFlashTestEnv::deleteBucket(::DB::S3::TiFlashS3Client & s3_client)
Expand Down

0 comments on commit 02bf222

Please sign in to comment.