Removing TODO and NOTE marks used mainly as hooks for review in the PR

This commit is contained in:
Jakub Kuklis 2021-12-09 18:11:03 +01:00
parent 26a6ef7138
commit 578aec97e9
5 changed files with 5 additions and 10 deletions

View File

@ -52,7 +52,7 @@ DiskBlobStorage::DiskBlobStorage(
std::shared_ptr<Azure::Storage::Blobs::BlobContainerClient> blob_container_client_,
SettingsPtr settings_,
GetDiskSettings settings_getter_) :
IDiskRemote(name_, "" /* TODO: shall we provide a config for this path? */, metadata_disk_, "DiskBlobStorage", settings_->thread_pool_size),
IDiskRemote(name_, "", metadata_disk_, "DiskBlobStorage", settings_->thread_pool_size),
blob_container_client(blob_container_client_),
current_settings(std::move(settings_)),
settings_getter(settings_getter_) {}
@ -132,7 +132,6 @@ bool DiskBlobStorage::checkUniqueId(const String & id) const
blobs_list_options.Prefix = id;
blobs_list_options.PageSizeHint = 1;
// TODO: does it return at most 5k blobs? Do we ever need the continuation token?
auto blobs_list_response = blob_container_client->ListBlobs(blobs_list_options);
auto blobs_list = blobs_list_response.Blobs;
@ -175,7 +174,7 @@ RemoteFSPathKeeperPtr DiskBlobStorage::createFSPathKeeper() const
return std::make_shared<BlobStoragePathKeeper>();
}
// NOTE: applyNewSettings - direct copy-paste from DiskS3
void DiskBlobStorage::applyNewSettings(const Poco::Util::AbstractConfiguration & config, ContextPtr context, const String &, const DisksMap &)
{
auto new_settings = settings_getter(config, "storage_configuration.disks." + name, context);

View File

@ -70,7 +70,6 @@ std::unique_ptr<DiskBlobStorageSettings> getSettings(const Poco::Util::AbstractC
config.getInt(config_prefix + ".max_single_read_retries", 3),
config.getInt(config_prefix + ".max_single_download_retries", 3),
config.getInt(config_prefix + ".thread_pool_size", 16)
// TODO: maybe use context for global settings from Settings.h
);
}

View File

@ -132,7 +132,7 @@ private:
std::shared_ptr<Azure::Storage::Blobs::BlobContainerClient> blob_container_client;
size_t max_single_read_retries;
size_t max_single_download_retries;
ReadSettings settings; // NOTE: used only for remote_fs_buffer_size
ReadSettings settings;
bool threadpool_read;
};
#endif

View File

@ -30,13 +30,12 @@ ReadBufferFromBlobStorage::ReadBufferFromBlobStorage(
size_t tmp_buffer_size_,
bool use_external_buffer_,
size_t read_until_position_)
// TODO: shall this notation be used in all constructors?
: SeekableReadBuffer(nullptr, 0)
, blob_container_client(blob_container_client_)
, path(path_)
, max_single_read_retries(max_single_read_retries_)
, max_single_download_retries(max_single_download_retries_)
, tmp_buffer_size(tmp_buffer_size_) // NOTE: field used only here in the constructor
, tmp_buffer_size(tmp_buffer_size_)
, use_external_buffer(use_external_buffer_)
, read_until_position(read_until_position_)
{
@ -73,7 +72,7 @@ bool ReadBufferFromBlobStorage::nextImpl()
size_t bytes_read = 0;
size_t sleep_time_with_backoff_milliseconds = 100;
for (size_t i = 0; i < max_single_read_retries; ++i) // TODO: are retries necessary with Azure reliable streams?
for (size_t i = 0; i < max_single_read_retries; ++i)
{
try
{

View File

@ -16,8 +16,6 @@ BLOB_STORAGE_DISK = "blob_storage_disk"
LOCAL_DISK = "hdd"
CONTAINER_NAME = "cont"
# TODO: these tests resemble S3 tests a lot, maybe they can be abstracted
@pytest.fixture(scope="module")
def cluster():