Skip to content

Commit bdf1132

Browse files
robot-clickhouserobot-clickhouse
committed
Backport #87109 to 25.3: EmbeddedRocksDB: Path must be inside user_files
1 parent a7ae14f commit bdf1132

16 files changed

+115
-29
lines changed

‎src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include <Common/Logger.h>
2929
#include <Common/logger_useful.h>
3030
#include <Common/Exception.h>
31+
#include <Common/filesystemHelpers.h>
32+
#include <Common/JSONBuilder.h>
3133
#include <Core/Settings.h>
3234
#include <Storages/AlterCommands.h>
3335
#include <Storages/RocksDB/RocksDBSettings.h>
@@ -207,10 +209,28 @@ StorageEmbeddedRocksDB::StorageEmbeddedRocksDB(const StorageID & table_id_,
207209
{
208210
setInMemoryMetadata(metadata_);
209211
setSettings(std::move(settings_));
212+
210213
if (rocksdb_dir.empty())
211214
{
212-
rocksdb_dir = context_->getPath() + relative_data_path_;
215+
/// We used to create databases under the database directory by default instead of user files. Check first if it exists there
216+
auto old_path = context_->getPath() + relative_data_path_;
217+
if (mode >= LoadingStrictnessLevel::ATTACH && fs::exists(old_path))
218+
rocksdb_dir = old_path;
219+
else
220+
rocksdb_dir = fs::path{getContext()->getUserFilesPath()} / relative_data_path_;
221+
}
222+
else
223+
{
224+
bool is_local = context_->getApplicationType() == Context::ApplicationType::LOCAL;
225+
fs::path user_files_path = is_local ? "" : fs::canonical(getContext()->getUserFilesPath());
226+
if (fs::path(rocksdb_dir).is_relative())
227+
rocksdb_dir = user_files_path / rocksdb_dir;
228+
rocksdb_dir = fs::absolute(rocksdb_dir).lexically_normal();
229+
230+
if (!is_local && !pathStartsWith(fs::path(rocksdb_dir), user_files_path))
231+
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path must be inside user-files path: {}", user_files_path.string());
213232
}
233+
214234
if (mode < LoadingStrictnessLevel::ATTACH)
215235
{
216236
fs::create_directories(rocksdb_dir);

‎tests/clickhouse-test‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,10 @@ class TestCase:
14711471
if tags and ("no-replicated-database" in tags) and args.replicated_database:
14721472
return FailureReason.REPLICATED_DB
14731473

1474+
# RocksDB tests cannot be used with replicated database as, in our CI, the databases share the same user_files
1475+
if tags and ("use-rocksdb" in tags) and args.replicated_database:
1476+
return FailureReason.REPLICATED_DB
1477+
14741478
if tags and ("no-distributed-cache" in tags) and args.distributed_cache:
14751479
return FailureReason.DISTRIBUTED_CACHE
14761480

‎tests/integration/test_backward_compatibility/test_rocksdb_upgrade.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import pytest
2+
3+
from helpers.cluster import CLICKHOUSE_CI_MIN_TESTED_VERSION, ClickHouseCluster
4+
5+
cluster = ClickHouseCluster(__file__)
6+
node = cluster.add_instance(
7+
"node",
8+
with_zookeeper=False,
9+
image="clickhouse/clickhouse-server",
10+
tag=CLICKHOUSE_CI_MIN_TESTED_VERSION,
11+
stay_alive=True,
12+
with_installed_binary=True,
13+
)
14+
15+
16+
@pytest.fixture(scope="module")
17+
def start_cluster():
18+
try:
19+
cluster.start()
20+
yield cluster
21+
22+
finally:
23+
cluster.shutdown()
24+
25+
26+
@pytest.fixture(autouse=True)
27+
def cleanup():
28+
yield
29+
node.restart_with_original_version(clear_data_dir=True)
30+
31+
32+
def test_rocksdb_upgrade(start_cluster):
33+
node.query(
34+
"""
35+
CREATE TABLE vt
36+
(
37+
`sha256` FixedString(64),
38+
`amount` Nullable(Int8)
39+
)
40+
ENGINE = EmbeddedRocksDB
41+
PRIMARY KEY sha256
42+
"""
43+
)
44+
45+
assert node.query("SELECT count() FROM vt") == "0\n"
46+
47+
node.restart_with_latest_version()
48+
49+
assert node.query("SELECT count() FROM vt") == "0\n"
50+
51+
node.query("""DROP TABLE vt""")

‎tests/integration/test_rocksdb_options/test.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ def test_valid_options(start_cluster):
4444
)
4545
node.query(
4646
"""
47-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only') PRIMARY KEY(key);
47+
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only_test_valid_options') PRIMARY KEY(key);
4848
DROP TABLE test;
4949
"""
5050
)
5151
node.query(
5252
"""
53-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/store/test_rocksdb_read_only', 1) PRIMARY KEY(key);
53+
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/user_files/test_rocksdb_read_only_test_valid_options', 1) PRIMARY KEY(key);
5454
DROP TABLE test;
5555
"""
5656
)

‎tests/integration/test_rocksdb_read_only/test.py‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,47 +28,47 @@ def test_read_only(start_cluster):
2828
with pytest.raises(QueryRuntimeException):
2929
node.query(
3030
"""
31-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only', 1) PRIMARY KEY(key);
31+
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only', 1) PRIMARY KEY(key);
3232
"""
3333
)
3434
# create directory if read_only = false
3535
node.query(
3636
"""
37-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only') PRIMARY KEY(key);
37+
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only') PRIMARY KEY(key);
3838
INSERT INTO test (key, value) VALUES (0, 'a'), (1, 'b'), (2, 'c');
3939
"""
4040
)
4141
# fail if create multiple non-read-only tables on the same directory
4242
with pytest.raises(QueryRuntimeException):
4343
node.query(
4444
"""
45-
CREATE TABLE test_fail (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only') PRIMARY KEY(key);
45+
CREATE TABLE test_fail (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only') PRIMARY KEY(key);
4646
"""
4747
)
4848
with pytest.raises(QueryRuntimeException):
4949
node.query(
5050
"""
51-
CREATE TABLE test_fail (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/store/test_rocksdb_read_only') PRIMARY KEY(key);
51+
CREATE TABLE test_fail (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/user_files/test_rocksdb_read_only') PRIMARY KEY(key);
5252
"""
5353
)
5454
# success if create multiple read-only tables on the same directory
5555
node.query(
5656
"""
57-
CREATE TABLE test_1 (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only', 1) PRIMARY KEY(key);
57+
CREATE TABLE test_1 (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only', 1) PRIMARY KEY(key);
5858
DROP TABLE test_1;
5959
"""
6060
)
6161
node.query(
6262
"""
63-
CREATE TABLE test_2 (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/store/test_rocksdb_read_only', 1) PRIMARY KEY(key);
63+
CREATE TABLE test_2 (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/user_files/test_rocksdb_read_only', 1) PRIMARY KEY(key);
6464
DROP TABLE test_2;
6565
"""
6666
)
6767
# success if create table on existing directory with no other tables on it
6868
node.query(
6969
"""
7070
DROP TABLE test;
71-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/store/test_rocksdb_read_only', 1) PRIMARY KEY(key);
71+
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(10, '/var/lib/clickhouse/user_files/test_rocksdb_read_only', 1) PRIMARY KEY(key);
7272
"""
7373
)
7474
result = node.query("""SELECT count() FROM test;""")
@@ -90,48 +90,48 @@ def test_dirctory_missing_after_stop(start_cluster):
9090
# for read_only = false
9191
node.query(
9292
"""
93-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only_missing') PRIMARY KEY(key);
93+
CREATE TABLE test_missing (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only_missing') PRIMARY KEY(key);
9494
"""
9595
)
9696
node.stop_clickhouse()
9797
node.exec_in_container(
9898
[
9999
"bash",
100100
"-c",
101-
"rm -r /var/lib/clickhouse/store/test_rocksdb_read_only_missing",
101+
"rm -r /var/lib/clickhouse/user_files/test_rocksdb_read_only_missing",
102102
]
103103
)
104104
node.start_clickhouse()
105105
result = node.query(
106-
"""INSERT INTO test (key, value) VALUES (0, 'a');
107-
SELECT * FROM test;
106+
"""INSERT INTO test_missing (key, value) VALUES (0, 'a');
107+
SELECT * FROM test_missing;
108108
"""
109109
)
110110
assert result.strip() == "0\ta"
111111
node.query(
112-
"""DROP TABLE test;
112+
"""DROP TABLE test_missing;
113113
"""
114114
)
115115
# for read_only = true
116116
node.query(
117117
"""
118-
CREATE TABLE test (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/store/test_rocksdb_read_only_missing', 1) PRIMARY KEY(key);
118+
CREATE TABLE test_missing (key UInt64, value String) Engine=EmbeddedRocksDB(0, '/var/lib/clickhouse/user_files/test_rocksdb_read_only_missing', 1) PRIMARY KEY(key);
119119
"""
120120
)
121121
node.stop_clickhouse()
122122
node.exec_in_container(
123123
[
124124
"bash",
125125
"-c",
126-
"rm -r /var/lib/clickhouse/store/test_rocksdb_read_only_missing",
126+
"rm -r /var/lib/clickhouse/user_files/test_rocksdb_read_only_missing",
127127
]
128128
)
129129
node.start_clickhouse()
130130
with pytest.raises(QueryRuntimeException):
131-
node.query("""INSERT INTO test (key, value) VALUES (1, 'b');""")
132-
result = node.query("""SELECT * FROM test;""")
131+
node.query("""INSERT INTO test_missing (key, value) VALUES (1, 'b');""")
132+
result = node.query("""SELECT * FROM test_missing;""")
133133
assert result.strip() == ""
134134
node.query(
135-
"""DROP TABLE test;
135+
"""DROP TABLE test_missing;
136136
"""
137137
)

‎tests/queries/0_stateless/01504_rocksdb.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Tags: no-ordinary-database, no-fasttest
1+
-- Tags: no-ordinary-database, no-fasttest, use-rocksdb
22
-- Tag no-ordinary-database: Sometimes cannot lock file most likely due to concurrent or adjacent tests, but we don't care how it works in Ordinary database
33
-- Tag no-fasttest: In fasttest, ENABLE_LIBRARIES=0, so rocksdb engine is not enabled by default
44

‎tests/queries/0_stateless/01686_rocksdb.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Tags: no-ordinary-database, no-fasttest
1+
-- Tags: no-ordinary-database, no-fasttest, use-rocksdb
22
-- Tag no-ordinary-database: Sometimes cannot lock file most likely due to concurrent or adjacent tests, but we don't care how it works in Ordinary database
33
-- Tag no-fasttest: In fasttest, ENABLE_LIBRARIES=0, so rocksdb engine is not enabled by default
44

‎tests/queries/0_stateless/01821_table_comment.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Tags: no-parallel, no-fasttest
1+
-- Tags: no-parallel, no-fasttest, use-rocksdb
22

33
DROP TABLE IF EXISTS t1;
44
DROP TABLE IF EXISTS t2;

‎tests/queries/0_stateless/02030_rocksdb_race_long.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env bash
2-
# Tags: race
2+
# Tags: race, use-rocksdb
33

44
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
55
# shellcheck source=../shell_config.sh

‎tests/queries/0_stateless/02375_rocksdb_with_filters.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env bash
2-
# Tags: no-fasttest
2+
# Tags: no-fasttest, use-rocksdb
33

44
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
55
# shellcheck source=../shell_config.sh

0 commit comments

Comments
 (0)