Skip to content

Commit

Permalink
Merge pull request ClickHouse#43343 from azat/disks/write-once
Browse files Browse the repository at this point in the history
Allow to "drop tables" from s3_plain disk (so as from web disk)
  • Loading branch information
kssenii authored Nov 20, 2022
2 parents 0b4e643 + 4f6703c commit c12cfab
Show file tree
Hide file tree
Showing 12 changed files with 47 additions and 8 deletions.
2 changes: 2 additions & 0 deletions src/Disks/DiskDecorator.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class DiskDecorator : public IDisk
bool checkUniqueId(const String & id) const override { return delegate->checkUniqueId(id); }
DataSourceDescription getDataSourceDescription() const override { return delegate->getDataSourceDescription(); }
bool isRemote() const override { return delegate->isRemote(); }
bool isReadOnly() const override { return delegate->isReadOnly(); }
bool isWriteOnce() const override { return delegate->isWriteOnce(); }
bool supportZeroCopyReplication() const override { return delegate->supportZeroCopyReplication(); }
bool supportParallelWrite() const override { return delegate->supportParallelWrite(); }
void onFreeze(const String & path) override;
Expand Down
2 changes: 2 additions & 0 deletions src/Disks/IDisk.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,8 @@ class IDisk : public Space

virtual bool isReadOnly() const { return false; }

virtual bool isWriteOnce() const { return false; }

/// Check if disk is broken. Broken disks will have 0 space and cannot be used.
virtual bool isBroken() const { return false; }

Expand Down
2 changes: 2 additions & 0 deletions src/Disks/ObjectStorages/Cached/CachedObjectStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ class CachedObjectStorage final : public IObjectStorage

bool isReadOnly() const override { return object_storage->isReadOnly(); }

bool isWriteOnce() const override { return object_storage->isWriteOnce(); }

const std::string & getCacheConfigName() const { return cache_config_name; }

ObjectStoragePtr getWrappedObjectStorage() { return object_storage; }
Expand Down
5 changes: 5 additions & 0 deletions src/Disks/ObjectStorages/DiskObjectStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,11 @@ bool DiskObjectStorage::isReadOnly() const
return object_storage->isReadOnly();
}

bool DiskObjectStorage::isWriteOnce() const
{
return object_storage->isWriteOnce();
}

DiskObjectStoragePtr DiskObjectStorage::createDiskObjectStorage()
{
return std::make_shared<DiskObjectStorage>(
Expand Down
6 changes: 6 additions & 0 deletions src/Disks/ObjectStorages/DiskObjectStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ friend class DiskObjectStorageRemoteMetadataRestoreHelper;
/// with static files, so only read-only operations are allowed for this storage.
bool isReadOnly() const override;

/// Is object write-once?
/// For example: S3PlainObjectStorage is write once, this means that it
/// does support BACKUP to this disk, but does not support INSERT into
/// MergeTree table on this disk.
bool isWriteOnce() const override;

/// Add a cache layer.
/// Example: DiskObjectStorage(S3ObjectStorage) -> DiskObjectStorage(CachedObjectStorage(S3ObjectStorage))
/// There can be any number of cache layers:
Expand Down
1 change: 1 addition & 0 deletions src/Disks/ObjectStorages/IObjectStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ class IObjectStorage
virtual bool supportsCache() const { return false; }

virtual bool isReadOnly() const { return false; }
virtual bool isWriteOnce() const { return false; }

virtual bool supportParallelWrite() const { return false; }

Expand Down
5 changes: 5 additions & 0 deletions src/Disks/ObjectStorages/S3/S3ObjectStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ class S3PlainObjectStorage : public S3ObjectStorage
{
data_source_description.type = DataSourceType::S3_Plain;
}

/// Notes:
/// - supports BACKUP to this disk
/// - does not support INSERT into MergeTree table on this disk
bool isWriteOnce() const override { return true; }
};

}
Expand Down
2 changes: 1 addition & 1 deletion src/Storages/IStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ bool IStorage::isStaticStorage() const
if (storage_policy)
{
for (const auto & disk : storage_policy->getDisks())
if (!disk->isReadOnly())
if (!(disk->isReadOnly() || disk->isWriteOnce()))
return false;
return true;
}
Expand Down
3 changes: 2 additions & 1 deletion src/Storages/IStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,8 @@ class IStorage : public std::enable_shared_from_this<IStorage>, public TypePromo
/// Returns storage policy if storage supports it.
virtual StoragePolicyPtr getStoragePolicy() const { return {}; }

/// Returns true if all disks of storage are read-only.
/// Returns true if all disks of storage are read-only or write-once.
/// NOTE: write-once also does not support INSERTs/merges/... for MergeTree
virtual bool isStaticStorage() const;

virtual bool supportsSubsetOfColumns() const { return false; }
Expand Down
16 changes: 16 additions & 0 deletions src/Storages/System/StorageSystemDisks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ StorageSystemDisks::StorageSystemDisks(const StorageID & table_id_)
{"keep_free_space", std::make_shared<DataTypeUInt64>()},
{"type", std::make_shared<DataTypeString>()},
{"is_encrypted", std::make_shared<DataTypeUInt8>()},
{"is_read_only", std::make_shared<DataTypeUInt8>()},
{"is_write_once", std::make_shared<DataTypeUInt8>()},
{"is_remote", std::make_shared<DataTypeUInt8>()},
{"is_broken", std::make_shared<DataTypeUInt8>()},
{"cache_path", std::make_shared<DataTypeString>()},
}));
setInMemoryMetadata(storage_metadata);
Expand All @@ -49,6 +53,10 @@ Pipe StorageSystemDisks::read(
MutableColumnPtr col_keep = ColumnUInt64::create();
MutableColumnPtr col_type = ColumnString::create();
MutableColumnPtr col_is_encrypted = ColumnUInt8::create();
MutableColumnPtr col_is_read_only = ColumnUInt8::create();
MutableColumnPtr col_is_write_once = ColumnUInt8::create();
MutableColumnPtr col_is_remote = ColumnUInt8::create();
MutableColumnPtr col_is_broken = ColumnUInt8::create();
MutableColumnPtr col_cache_path = ColumnString::create();

for (const auto & [disk_name, disk_ptr] : context->getDisksMap())
Expand All @@ -62,6 +70,10 @@ Pipe StorageSystemDisks::read(
auto data_source_description = disk_ptr->getDataSourceDescription();
col_type->insert(toString(data_source_description.type));
col_is_encrypted->insert(data_source_description.is_encrypted);
col_is_read_only->insert(disk_ptr->isReadOnly());
col_is_write_once->insert(disk_ptr->isWriteOnce());
col_is_remote->insert(disk_ptr->isRemote());
col_is_broken->insert(disk_ptr->isBroken());

String cache_path;
if (disk_ptr->supportsCache())
Expand All @@ -79,6 +91,10 @@ Pipe StorageSystemDisks::read(
res_columns.emplace_back(std::move(col_keep));
res_columns.emplace_back(std::move(col_type));
res_columns.emplace_back(std::move(col_is_encrypted));
res_columns.emplace_back(std::move(col_is_read_only));
res_columns.emplace_back(std::move(col_is_write_once));
res_columns.emplace_back(std::move(col_is_remote));
res_columns.emplace_back(std::move(col_is_broken));
res_columns.emplace_back(std::move(col_cache_path));

UInt64 num_rows = res_columns.at(0)->size();
Expand Down
7 changes: 1 addition & 6 deletions tests/integration/test_attach_backup_from_s3_plain/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ def start_cluster():
pytest.param("wide", "backup_wide", "s3_backup_wide", int(0), id="wide"),
],
)
def test_attach_compact_part(
table_name, backup_name, storage_policy, min_bytes_for_wide_part
):
def test_attach_part(table_name, backup_name, storage_policy, min_bytes_for_wide_part):
node.query(
f"""
-- Catch any errors (NOTE: warnings are ok)
Expand Down Expand Up @@ -61,9 +59,6 @@ def test_attach_compact_part(

node.query(
f"""
-- NOTE: be aware not to DROP the table, but DETACH first to keep it in S3.
detach table ordinary_db.{table_name};
-- NOTE: DROP DATABASE cannot be done w/o this due to metadata leftovers
set force_remove_data_recursively_on_drop=1;
drop database ordinary_db sync;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ CREATE TABLE system.disks
`keep_free_space` UInt64,
`type` String,
`is_encrypted` UInt8,
`is_read_only` UInt8,
`is_write_once` UInt8,
`is_remote` UInt8,
`is_broken` UInt8,
`cache_path` String
)
ENGINE = SystemDisks
Expand Down

0 comments on commit c12cfab

Please sign in to comment.