Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Commit

Permalink
Merge pull request #250 from irq0/pr/connpool-new
Browse files Browse the repository at this point in the history
  • Loading branch information
Marcel Lauhoff authored Nov 22, 2023
2 parents d4b05d4 + 1c2087f commit 9dcbde6
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 11 deletions.
28 changes: 24 additions & 4 deletions src/rgw/driver/sfs/sqlite/retry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <system_error>
#include <thread>

#include "dbapi.h"
#include "errors.h"
#include "rgw_perf_counters.h"

Expand All @@ -39,7 +40,7 @@ class RetrySQLiteBusy {
const int m_max_retries{10};
bool m_successful{false};
int m_retries{0};
std::error_code m_failed_error{};
int m_failed_sqlite_error{};

public:
RetrySQLiteBusy(Func&& fn) : m_fn(std::forward<Func>(fn)) {}
Expand All @@ -60,11 +61,13 @@ class RetrySQLiteBusy {
try {
Return result = m_fn();
m_successful = true;
m_failed_error = std::error_code{};
m_failed_sqlite_error = SQLITE_OK;
m_retries = retry;
return result;
// TODO(https://github.com/aquarist-labs/s3gw/issues/788) Remove
// sqlite_orm path
} catch (const std::system_error& ex) {
m_failed_error = ex.code();
m_failed_sqlite_error = ex.code().value();
if (critical_error(ex.code().value())) {
ceph_abort_msgf(
"Critical SQLite error %d. Shutting down.", ex.code().value()
Expand All @@ -80,6 +83,23 @@ class RetrySQLiteBusy {
if (perfcounter) {
perfcounter->inc(l_rgw_sfs_sqlite_retry_retried_count, 1);
}
} catch (const dbapi::sqlite::sqlite_exception& ex) {
m_failed_sqlite_error = ex.get_code();
if (critical_error(ex.get_code())) {
ceph_abort_msgf(
"Critical SQLite error %d. Shutting down.", ex.get_code()
);
}
if (!busy_error(ex.get_code())) {
// Rethrow, expect a higher layer to handle (e.g constraint
// violations), reply internal server error or shut us down
throw ex;
}
std::this_thread::sleep_for(10ms * retry);
m_retries = retry;
if (perfcounter) {
perfcounter->inc(l_rgw_sfs_sqlite_retry_retried_count, 1);
}
}
}
m_successful = false;
Expand All @@ -94,7 +114,7 @@ class RetrySQLiteBusy {
bool successful() { return m_successful; };
/// failed_error returns the non-critical error code of the last
/// failed attempt to run fn
std::error_code failed_error() { return m_failed_error; };
int failed_error() { return m_failed_sqlite_error; };
/// retries returns the number of retries to failure or success
int retries() { return m_retries; };
};
Expand Down
85 changes: 78 additions & 7 deletions src/test/rgw/sfs/test_rgw_sfs_retry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <gtest/gtest.h>

#include "common/ceph_context.h"
#include "driver/sfs/sqlite/dbapi.h"
#include "driver/sfs/sqlite/retry.h"
#include "driver/sfs/sqlite/sqlite_orm.h"
#include "gtest/gtest.h"
Expand All @@ -26,9 +27,13 @@ class TestSFSRetrySQLite : public ::testing::Test {
void TearDown() override {}
};

class TestSFSRetrySQLiteDeathTest : public TestSFSRetrySQLite {};
// TODO(https://github.com/aquarist-labs/s3gw/issues/788) Remove
// TestSFSRetrySQLiteOrm tests

TEST_F(TestSFSRetrySQLite, retry_non_crit_till_failure) {
class TestSFSRetrySQLiteOrm : public TestSFSRetrySQLite {};
class TestSFSRetrySQLiteOrmDeathTest : public TestSFSRetrySQLiteOrm {};

TEST_F(TestSFSRetrySQLiteOrm, retry_non_crit_till_failure) {
auto exception =
std::system_error{SQLITE_BUSY, sqlite_orm::get_sqlite_error_category()};
RetrySQLiteBusy<int> uut([&]() {
Expand All @@ -37,11 +42,11 @@ TEST_F(TestSFSRetrySQLite, retry_non_crit_till_failure) {
});
EXPECT_EQ(uut.run(), std::nullopt);
EXPECT_FALSE(uut.successful());
EXPECT_EQ(uut.failed_error(), exception.code());
EXPECT_EQ(uut.failed_error(), exception.code().value());
EXPECT_GT(uut.retries(), 0);
}

TEST_F(TestSFSRetrySQLiteDeathTest, crit_aborts) {
TEST_F(TestSFSRetrySQLiteOrmDeathTest, crit_aborts) {
GTEST_FLAG_SET(death_test_style, "threadsafe");
auto exception = std::system_error{
SQLITE_CORRUPT, sqlite_orm::get_sqlite_error_category()
Expand All @@ -53,14 +58,14 @@ TEST_F(TestSFSRetrySQLiteDeathTest, crit_aborts) {
ASSERT_DEATH(uut.run(), "Critical SQLite error");
}

TEST_F(TestSFSRetrySQLite, simple_return_succeeds_immediately) {
TEST_F(TestSFSRetrySQLiteOrm, simple_return_succeeds_immediately) {
RetrySQLiteBusy<int> uut([&]() { return 42; });
EXPECT_EQ(uut.run(), 42);
EXPECT_TRUE(uut.successful());
EXPECT_EQ(uut.retries(), 0);
}

TEST_F(TestSFSRetrySQLite, retry_second_time_success) {
TEST_F(TestSFSRetrySQLiteOrm, retry_second_time_success) {
auto exception =
std::system_error{SQLITE_BUSY, sqlite_orm::get_sqlite_error_category()};
bool first = true;
Expand All @@ -74,6 +79,72 @@ TEST_F(TestSFSRetrySQLite, retry_second_time_success) {
});
EXPECT_EQ(uut.run(), 23);
EXPECT_TRUE(uut.successful());
EXPECT_NE(uut.failed_error(), exception.code());
EXPECT_NE(uut.failed_error(), exception.code().value());
EXPECT_EQ(uut.retries(), 1);
}

class TestSFSRetrySQLiteDeathTest : public TestSFSRetrySQLite {};

TEST_F(TestSFSRetrySQLite, retry_non_crit_till_failure) {
auto exception = rgw::sal::sfs::dbapi::sqlite::errors::busy(SQLITE_BUSY, "");
RetrySQLiteBusy<int> uut([&]() {
throw exception;
return 0;
});
EXPECT_EQ(uut.run(), std::nullopt);
EXPECT_FALSE(uut.successful());
EXPECT_EQ(uut.failed_error(), exception.get_code());
EXPECT_GT(uut.retries(), 0);
}

TEST_F(TestSFSRetrySQLite, retry_non_crit_extended_till_failure) {
auto exception = rgw::sal::sfs::dbapi::sqlite::errors::busy_snapshot(
SQLITE_BUSY_SNAPSHOT, ""
);
RetrySQLiteBusy<int> uut([&]() {
throw exception;
return 0;
});
EXPECT_EQ(uut.run(), std::nullopt);
EXPECT_FALSE(uut.successful());
EXPECT_EQ(uut.failed_error(), exception.get_code());
EXPECT_GT(uut.retries(), 0);
}

TEST_F(TestSFSRetrySQLiteDeathTest, crit_aborts) {
GTEST_FLAG_SET(death_test_style, "threadsafe");
auto exception = std::system_error{
SQLITE_CORRUPT, sqlite_orm::get_sqlite_error_category()
};
RetrySQLiteBusy<int> uut([&]() {
rgw::sal::sfs::dbapi::sqlite::errors::throw_sqlite_error(SQLITE_CORRUPT);
return 0;
});
ASSERT_DEATH(uut.run(), "Critical SQLite error");
}

TEST_F(TestSFSRetrySQLite, simple_return_succeeds_immediately) {
RetrySQLiteBusy<int> uut([&]() { return 42; });
EXPECT_EQ(uut.run(), 42);
EXPECT_TRUE(uut.successful());
EXPECT_EQ(uut.retries(), 0);
}

TEST_F(TestSFSRetrySQLite, retry_second_time_success) {
auto exception = rgw::sal::sfs::dbapi::sqlite::sqlite_exception(
SQLITE_BUSY, "", "non critical error"
);
bool first = true;
RetrySQLiteBusy<int> uut([&]() {
if (first) {
first = false;
throw exception;
} else {
return 23;
}
});
EXPECT_EQ(uut.run(), 23);
EXPECT_TRUE(uut.successful());
EXPECT_NE(uut.failed_error(), exception.get_code());
EXPECT_EQ(uut.retries(), 1);
}

0 comments on commit 9dcbde6

Please sign in to comment.