Skip to content

Commit

Permalink
[BugFix]don't set NETWORK_CONNECTION error when received response fro…
Browse files Browse the repository at this point in the history
…m oss (#52569)

Signed-off-by: zombee0 <[email protected]>
  • Loading branch information
zombee0 authored Nov 5, 2024
1 parent 7c45354 commit 868f415
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 11 deletions.
13 changes: 2 additions & 11 deletions be/src/fs/s3/poco_http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,8 @@ void PocoHttpClient::MakeRequestInternal(Aws::Http::HttpRequest& request,
// pass
// TODO, throttling
}
if (status_code >= ResponseCode::SUCCESS_RESPONSE_MIN &&
status_code <= ResponseCode::SUCCESS_RESPONSE_MAX) {
if (!poco_response.isBodyFilled()) {
Poco::StreamCopier::copyStream(response_body_stream, response->GetResponseBody());
}
} else {
std::string error_message;
Poco::StreamCopier::copyToString(response_body_stream, error_message);

response->SetClientErrorType(Aws::Client::CoreErrors::NETWORK_CONNECTION);
response->SetClientErrorMessage(error_message);
if (!poco_response.isBodyFilled()) {
Poco::StreamCopier::copyStream(response_body_stream, response->GetResponseBody());
}
}

Expand Down
26 changes: 26 additions & 0 deletions be/test/fs/poco_http_client_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,30 @@ TEST_F(PocoHttpClientTest, TestErrorAkSk) {
EXPECT_TRUE(r.status().message().find("SdkResponseCode=403") != std::string::npos);
}

TEST_F(PocoHttpClientTest, TestNotFoundKey) {
Aws::Client::ClientConfiguration config;
config.endpointOverride = config::object_storage_endpoint.empty() ? getenv("STARROCKS_UT_S3_ENDPOINT")
: config::object_storage_endpoint;
// Create a custom retry strategy
int maxRetries = 2;
long scaleFactor = 25;
std::shared_ptr<Aws::Client::RetryStrategy> retryStrategy =
std::make_shared<Aws::Client::DefaultRetryStrategy>(maxRetries, scaleFactor);

// Create a client configuration object and set the custom retry strategy
config.retryStrategy = retryStrategy;

auto credentials = std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(ak, sk);

auto client = std::make_shared<Aws::S3::S3Client>(std::move(credentials), std::move(config),
Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never, true);

auto stream = std::make_unique<starrocks::io::S3InputStream>(client, s_bucket_name, "not_found_key");
char buf[6];
auto r = stream->read(buf, sizeof(buf));
EXPECT_TRUE(r.status().message().find("SdkResponseCode=404") != std::string::npos);
// ErrorCode 16 means RESOURCE_NOT_FOUND
EXPECT_TRUE(r.status().message().find("SdkErrorType=16") != std::string::npos);
}

} // namespace starrocks::poco
9 changes: 9 additions & 0 deletions be/test/io/s3_input_stream_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ TEST_F(S3InputStreamTest, test_read) {
ASSERT_EQ(10, *f->position());
}

TEST_F(S3InputStreamTest, test_not_found) {
auto f = std::make_unique<S3InputStream>(g_s3client, s_bucket_name, "key_not_found");
char buf[6];
auto r = f->read(buf, sizeof(buf));
EXPECT_TRUE(r.status().message().find("SdkResponseCode=404") != std::string::npos);
// ErrorCode 16 means RESOURCE_NOT_FOUND
EXPECT_TRUE(r.status().message().find("SdkErrorType=16") != std::string::npos);
}

TEST_F(S3InputStreamTest, test_skip) {
auto f = new_random_access_file();
char buf[6];
Expand Down

0 comments on commit 868f415

Please sign in to comment.