From 1d6d20f3cba486972ecfece43c4d0a726808930a Mon Sep 17 00:00:00 2001 From: Dmitri Petrov Date: Thu, 21 Dec 2023 23:44:08 +0000 Subject: [PATCH] Use virtual inheritance in FileSystem hierarchy. --- cloud/cloud_file_system.cc | 6 +++--- cloud/db_cloud_test.cc | 8 ++++---- db/db_test_util.cc | 4 ++-- include/rocksdb/cloud/cloud_file_system.h | 2 +- include/rocksdb/cloud/cloud_file_system_impl.h | 1 - include/rocksdb/customizable.h | 2 +- include/rocksdb/file_system.h | 2 +- 7 files changed, 12 insertions(+), 13 deletions(-) diff --git a/cloud/cloud_file_system.cc b/cloud/cloud_file_system.cc index 8d659931e0f0..d8d11d1abc69 100644 --- a/cloud/cloud_file_system.cc +++ b/cloud/cloud_file_system.cc @@ -547,7 +547,7 @@ Status CloudFileSystem::CreateFromString( } if (s.ok()) { - result->reset(static_cast(fs.release())); + result->reset(dynamic_cast(fs.release())); } return s; @@ -603,7 +603,7 @@ Status CloudFileSystem::CreateFromString( } if (s.ok()) { - result->reset(static_cast(fs.release())); + result->reset(dynamic_cast(fs.release())); } return s; @@ -628,7 +628,7 @@ Status CloudFileSystem::NewAwsFileSystem( Status st = AwsFileSystem::NewAwsFileSystem(base_fs, options, logger, cfs); if (st.ok()) { // store a copy of the logger - auto* cloud = static_cast(*cfs); + auto* cloud = dynamic_cast(*cfs); cloud->info_log_ = logger; // start the purge thread only if there is a destination bucket diff --git a/cloud/db_cloud_test.cc b/cloud/db_cloud_test.cc index 71a513a28fa5..b7839a424b66 100644 --- a/cloud/db_cloud_test.cc +++ b/cloud/db_cloud_test.cc @@ -353,11 +353,11 @@ class CloudTest : public testing::Test { CloudFileSystem* GetCloudFileSystem() const { EXPECT_TRUE(aenv_); - return static_cast(aenv_->GetFileSystem().get()); + return dynamic_cast(aenv_->GetFileSystem().get()); } CloudFileSystemImpl* GetCloudFileSystemImpl() const { EXPECT_TRUE(aenv_); - return static_cast(aenv_->GetFileSystem().get()); + return dynamic_cast(aenv_->GetFileSystem().get()); } DBImpl* GetDBImpl() const { @@ -861,7 +861,7 @@ TEST_F(CloudTest, DISABLED_TrueClone) { ASSERT_TRUE(value.compare("World") == 0); // Assert that there are no redundant sst files - auto* cimpl = static_cast(env->GetFileSystem().get()); + auto* cimpl = dynamic_cast(env->GetFileSystem().get()); std::vector to_be_deleted; ASSERT_OK( cimpl->FindObsoleteFiles(cimpl->GetSrcBucketName(), &to_be_deleted)); @@ -1063,7 +1063,7 @@ TEST_F(CloudTest, Savepoint) { cloud_db->GetLiveFilesMetaData(&flist); ASSERT_TRUE(flist.size() == 1); - auto* cimpl = static_cast(env->GetFileSystem().get()); + auto* cimpl = dynamic_cast(env->GetFileSystem().get()); auto remapped_fname = cimpl->RemapFilename(flist[0].name); // source path std::string spath = cimpl->GetSrcObjectPath() + "/" + remapped_fname; diff --git a/db/db_test_util.cc b/db/db_test_util.cc index 149768efb6c9..d93aa8cc791b 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -157,7 +157,7 @@ DBTestBase::~DBTestBase() { #ifndef ROCKSDB_LITE #ifdef USE_AWS - auto* cfs = static_cast(s3_env_->GetFileSystem().get()); + auto* cfs = dynamic_cast(s3_env_->GetFileSystem().get()); cfs->GetStorageProvider()->EmptyBucket(cfs->GetSrcBucketName(), cfs->GetSrcObjectPath()); #endif // USE_AWS @@ -826,7 +826,7 @@ void DBTestBase::Destroy(const Options& options, bool delete_cf_paths) { ASSERT_OK(DestroyDB(dbname_, options, column_families)); #ifdef USE_AWS if (s3_env_) { - auto* cfs = static_cast(s3_env_->GetFileSystem().get()); + auto* cfs = dynamic_cast(s3_env_->GetFileSystem().get()); auto st = cfs->GetStorageProvider()->EmptyBucket(cfs->GetSrcBucketName(), dbname_); ASSERT_TRUE(st.ok() || st.IsNotFound()); diff --git a/include/rocksdb/cloud/cloud_file_system.h b/include/rocksdb/cloud/cloud_file_system.h index 42aa07c8aaef..7cae24f4b3e1 100644 --- a/include/rocksdb/cloud/cloud_file_system.h +++ b/include/rocksdb/cloud/cloud_file_system.h @@ -494,7 +494,7 @@ struct CloudManifestDelta { // NOTE: The AWS SDK must be initialized before the CloudFileSystem is // constructed, and remain active (Aws::ShutdownAPI() not called) as long as any // CloudFileSystem objects exist. -class CloudFileSystem : public FileSystem { +class CloudFileSystem : public virtual FileSystem { protected: CloudFileSystemOptions cloud_fs_options; std::shared_ptr base_fs_; // The underlying file system diff --git a/include/rocksdb/cloud/cloud_file_system_impl.h b/include/rocksdb/cloud/cloud_file_system_impl.h index dcd9ffc44522..811485125b10 100644 --- a/include/rocksdb/cloud/cloud_file_system_impl.h +++ b/include/rocksdb/cloud/cloud_file_system_impl.h @@ -236,7 +236,6 @@ class CloudFileSystemImpl : public CloudFileSystem { "CloudFileSystemImpl::IsDirectory() not supported."); } - IOStatus Poll(std::vector& io_handles, size_t /*min_completions*/) override { for (auto* handle : io_handles) { diff --git a/include/rocksdb/customizable.h b/include/rocksdb/customizable.h index 92f7504ae1bb..8cf34e2502cf 100644 --- a/include/rocksdb/customizable.h +++ b/include/rocksdb/customizable.h @@ -135,7 +135,7 @@ class Customizable : public Configurable { template T* CheckedCast() { if (IsInstanceOf(T::kClassName())) { - return static_cast(this); + return dynamic_cast(this); } else { auto inner = const_cast(Inner()); if (inner != nullptr) { diff --git a/include/rocksdb/file_system.h b/include/rocksdb/file_system.h index 05ac40b41a89..22e07bb3536b 100644 --- a/include/rocksdb/file_system.h +++ b/include/rocksdb/file_system.h @@ -1380,7 +1380,7 @@ class FSDirectory { // An implementation of Env that forwards all calls to another Env. // May be useful to clients who wish to override just part of the // functionality of another Env. -class FileSystemWrapper : public FileSystem { +class FileSystemWrapper : public virtual FileSystem { public: // Initialize an EnvWrapper that delegates all calls to *t explicit FileSystemWrapper(const std::shared_ptr& t);