Merge pull request #62 from rockset/CloudEnvOptions

Make CloudEnvOptions a member variable of CloudEnv

Author: igorcanadi

cloud/aws/aws_env.cc

@@ -261,10 +261,9 @@ Aws::S3::Model::HeadObjectOutcome AwsS3ClientWrapper::HeadObject(
 AwsEnv::AwsEnv(Env* underlying_env,
                const CloudEnvOptions& _cloud_env_options,
                const std::shared_ptr<Logger> & info_log)
-    : CloudEnvImpl(_cloud_env_options.cloud_type, _cloud_env_options.log_type,
+    : CloudEnvImpl(_cloud_env_options,
                    underlying_env),
       info_log_(info_log),
-      cloud_env_options(_cloud_env_options),
       running_(true),
       has_src_bucket_(false),
       has_dest_bucket_(false),

cloud/aws/aws_env.h

@@ -241,24 +241,8 @@ class AwsEnv : public CloudEnvImpl {
 
   bool IsRunning() const { return running_; }
 
-  const std::string& GetSrcBucketName() const override {
-    return cloud_env_options.src_bucket.GetBucketName();
-  }
-  const std::string& GetSrcObjectPath() const override {
-    return cloud_env_options.src_bucket.GetObjectPath();
-  }
 
-  const std::string& GetDestBucketName() const override {
-    return cloud_env_options.dest_bucket.GetBucketName();
-  }
-  const std::string& GetDestObjectPath() const override {
-    return cloud_env_options.dest_bucket.GetObjectPath();
-  }
 
-  const CloudEnvOptions& GetCloudEnvOptions() override {
-    return cloud_env_options;
-  }
-
   std::string GetWALCacheDir();
 
   std::shared_ptr<Logger> info_log_;  // informational messages
@@ -269,9 +253,6 @@ class AwsEnv : public CloudEnvImpl {
   // AWS's utility to help out with uploading and downloading S3 file
   std::shared_ptr<Aws::Transfer::TransferManager> awsTransferManager_;
 
-  // Configurations for this cloud environent
-  CloudEnvOptions cloud_env_options;
-
   //
   // Get credentials for running unit tests
   //

cloud/aws/aws_s3.cc

@@ -302,7 +302,7 @@ Status S3WritableFile::Close() {
   }
 
   // delete local file
-  if (!env_->cloud_env_options.keep_local_sst_files) {
+  if (!env_->GetCloudEnvOptions().keep_local_sst_files) {
     status_ = env_->GetPosixEnv()->DeleteFile(fname_);
     if (!status_.ok()) {
       Log(InfoLogLevel::ERROR_LEVEL, env_->info_log_,

cloud/cloud_env.cc

@@ -88,9 +88,8 @@ CloudEnv::~CloudEnv() {}
 
 CloudEnvWrapper::~CloudEnvWrapper() {}
 
-CloudEnvImpl::CloudEnvImpl(
-      CloudType cloud_type, LogType log_type, Env* base_env)
-    : cloud_type_(cloud_type), log_type_(log_type),
+CloudEnvImpl::CloudEnvImpl(const CloudEnvOptions& opts, Env* base_env)
+  : CloudEnv(opts),
       base_env_(base_env), purger_is_running_(true) {}
 
 CloudEnvImpl::~CloudEnvImpl() { StopPurger(); }
@@ -242,7 +241,7 @@ Status CloudEnv::NewAwsEnv(Env* base_env,
   // If the src bucket is not specified, then this is a pass-through cloud env.
   if (! options.dest_bucket.IsValid() &&
       ! options.src_bucket.IsValid()) {
-    *cenv = new CloudEnvWrapper(base_env);
+    *cenv = new CloudEnvWrapper(options, base_env);
     return Status::OK();
   }
 

cloud/cloud_env_impl.h

@@ -20,12 +20,11 @@ class CloudEnvImpl : public CloudEnv {
 
  public:
   // Constructor
-  CloudEnvImpl(CloudType cloud_type, LogType log_type, Env* base_env);
+  CloudEnvImpl(const CloudEnvOptions & options, Env* base_env);
 
   virtual ~CloudEnvImpl();
 
-  const CloudType& GetCloudType() const { return cloud_type_; }
-  const LogType& GetLogType() const { return log_type_; }
+  const CloudType& GetCloudType() const { return cloud_env_options.cloud_type; }
 
   // Returns the underlying env
   Env* GetBaseEnv() override { return base_env_; }
@@ -98,11 +97,6 @@ class CloudEnvImpl : public CloudEnv {
   }
 
  protected:
-  // The type of cloud service e.g. AWS, Azure, Google,  etc.
-  const CloudType cloud_type_;
-
-  // The type of cloud service used for WAL e.g. Kinesis, Kafka, etc.
-  const LogType log_type_;
 
   // The dbid of the source database that is cloned
   std::string src_dbid_;

cloud/cloud_env_wrapper.h

@@ -19,8 +19,9 @@ namespace rocksdb {
 class CloudEnvWrapper : public CloudEnvImpl {
  public:
   // Initialize an EnvWrapper that delegates all calls to *t
-  explicit CloudEnvWrapper(Env* t) : CloudEnvImpl(
-      CloudType::kCloudNone, LogType::kLogNone, t) {
+  explicit CloudEnvWrapper(const CloudEnvOptions& options, Env* t) : CloudEnvImpl(options, t) {
+    cloud_env_options.log_type = LogType::kLogNone;
+    cloud_env_options.cloud_type = CloudType::kCloudNone;
     notsup_ = Status::NotSupported();
   }
 
@@ -54,11 +55,6 @@ class CloudEnvWrapper : public CloudEnvImpl {
     return notsup_;
   }
 
-  virtual const std::string& GetSrcBucketName() const override { return empty_; }
-  virtual const std::string& GetSrcObjectPath() const override { return empty_; }
-  virtual const std::string& GetDestBucketName() const override { return empty_; }
-  virtual const std::string& GetDestObjectPath() const override { return empty_; }
-
   // Ability to read a file directly from cloud storage
   virtual Status NewSequentialFileCloud(const std::string& fname,
                                         unique_ptr<SequentialFile>* result,
@@ -208,7 +204,6 @@ class CloudEnvWrapper : public CloudEnvImpl {
 
   uint64_t GetThreadID() const override { return base_env_->GetThreadID(); }
 
-  const CloudEnvOptions& GetCloudEnvOptions() override { return options_; }
 
   Status ListObjects(const std::string& bucket_name_prefix,
                      const std::string& bucket_object_prefix,
@@ -251,7 +246,6 @@ class CloudEnvWrapper : public CloudEnvImpl {
  private:
   Status notsup_;
   std::string empty_;
-  const CloudEnvOptions options_;
 };
 
 #pragma GCC diagnostic pop

include/rocksdb/cloud/cloud_env_options.h

@@ -247,6 +247,9 @@ typedef std::map<std::string, std::string> DbidList;
 // The Cloud environment
 //
 class CloudEnv : public Env {
+ protected:
+  CloudEnvOptions cloud_env_options;
+  CloudEnv(const CloudEnvOptions& options) : cloud_env_options(options) { }
  public:
   // Returns the underlying env
   virtual Env* GetBaseEnv() = 0;
@@ -277,17 +280,28 @@ class CloudEnv : public Env {
   // GetSrcObjectPath specifies the path inside that bucket
   // where data files reside. The specified bucket is used in
   // a readonly mode by the associated DBCloud instance.
-  virtual const std::string& GetSrcBucketName() const = 0;
-  virtual const std::string& GetSrcObjectPath() const = 0;
+  const std::string& GetSrcBucketName() const {
+    return cloud_env_options.src_bucket.GetBucketName();
+  }
+  const std::string& GetSrcObjectPath() const {
+    return cloud_env_options.src_bucket.GetObjectPath();
+  }
 
   // The DestBucketName identifies the cloud storage bucket and
   // GetDestObjectPath specifies the path inside that bucket
   // where data files reside. The associated DBCloud instance
   // writes newly created files to this bucket.
-  virtual const std::string& GetDestBucketName() const = 0;
-  virtual const std::string& GetDestObjectPath() const = 0;
+  const std::string& GetDestBucketName() const {
+    return cloud_env_options.dest_bucket.GetBucketName();
+  }
+  const std::string& GetDestObjectPath() const {
+    return cloud_env_options.dest_bucket.GetObjectPath();
+  }
+
   // returns the options used to create this env
-  virtual const CloudEnvOptions& GetCloudEnvOptions() = 0;
+  const CloudEnvOptions& GetCloudEnvOptions() const {
+    return cloud_env_options;
+  }
 
   // returns all the objects that have the specified path prefix and
   // are stored in a cloud bucket