diff --git a/dbms/src/Dictionaries/CacheDictionary.inc.h b/dbms/src/Dictionaries/CacheDictionary.inc.h index d102ad5659830a90fae3d6e19d2311724dba433c..9b6e7de747e21db4305537d5e6c09812f0b8e680 100644 --- a/dbms/src/Dictionaries/CacheDictionary.inc.h +++ b/dbms/src/Dictionaries/CacheDictionary.inc.h @@ -361,30 +361,26 @@ void CacheDictionary::prepareAnswer( continue; } - /// TODO: understand properly this code and remove -// if (error_count) -// { -// if (find_result.outdated) -// { -// /// We have expired data for that `id` so we can continue using it. -// bool was_default = cell.isDefault(); -// -// if (was_default) -// LOG_FATAL(log, "WAS DEFAULT"); -// -// cell.setExpiresAt(backoff_end_time); -// if (was_default) -// cell.setDefault(); -// -// if (was_default) -// on_id_not_found(id, cell_idx); -// else -// on_cell_updated(id, cell_idx); -// continue; -// } -// /// We don't have expired data for that `id` so all we can do is to rethrow `last_exception`. -// std::rethrow_exception(last_exception); -// } + if (error_count) + { + if (find_result.outdated) + { + /// We have expired data for that `id` so we can continue using it. + bool was_default = cell.isDefault(); + + cell.setExpiresAt(backoff_end_time); + if (was_default) + cell.setDefault(); + + if (was_default) + on_id_not_found(id, cell_idx); + else + on_cell_updated(id, cell_idx); + continue; + } + /// We don't have expired data for that `id` so all we can do is to rethrow `last_exception`. + std::rethrow_exception(last_exception); + } /// Check if cell had not been occupied before and increment element counter if it hadn't if (cell.id == 0 && cell_idx != zero_cell_idx) diff --git a/dbms/tests/integration/helpers/cluster.py b/dbms/tests/integration/helpers/cluster.py index 6e7ffa7a04bff54adb5efc29ba5c5e857f1454e6..c70a61c4dbafa661eb26e26145e288b835473309 100644 --- a/dbms/tests/integration/helpers/cluster.py +++ b/dbms/tests/integration/helpers/cluster.py @@ -650,6 +650,15 @@ class ClickHouseInstance: self.exec_in_container(["bash", "-c", "kill -9 {}".format(pid)], user='root') time.sleep(stop_start_wait_sec) + def restore_clickhouse(self, retries=100): + pid = self.get_process_pid("clickhouse") + if pid: + raise Exception("ClickHouse has already started") + self.exec_in_container(["bash", "-c", "{} --daemon".format(CLICKHOUSE_START_COMMAND)], user=str(os.getuid())) + from helpers.test_tools import assert_eq_with_retry + # wait start + assert_eq_with_retry(self, "select 1", "1", retry_count=retries) + def restart_clickhouse(self, stop_start_wait_sec=5, kill=False): if not self.stay_alive: raise Exception("clickhouse can be restarted only with stay_alive=True instance") @@ -956,3 +965,14 @@ class ClickHouseInstance: def destroy_dir(self): if p.exists(self.path): shutil.rmtree(self.path) + + +class ClickHouseKiller(object): + def __init__(self, clickhouse_node): + self.clickhouse_node = clickhouse_node + + def __enter__(self): + self.clickhouse_node.kill_clickhouse() + + def __exit__(self, exc_type, exc_val, exc_tb): + self.clickhouse_node.restore_clickhouse() \ No newline at end of file diff --git a/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_default_reading.py b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_default_reading.py new file mode 100644 index 0000000000000000000000000000000000000000..8da882679bd5e5b70591ada9c78be07b90b858b0 --- /dev/null +++ b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_default_reading.py @@ -0,0 +1,64 @@ +from __future__ import print_function +import pytest +import time +import os +from contextlib import contextmanager + +from helpers.cluster import ClickHouseCluster +from helpers.cluster import ClickHouseKiller +from helpers.network import PartitionManager + +SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) +cluster = ClickHouseCluster(__file__, base_configs_dir=os.path.join(SCRIPT_DIR, 'configs')) + +dictionary_node = cluster.add_instance('dictionary_node', stay_alive=True) +main_node = cluster.add_instance('main_node', main_configs=['configs/dictionaries/cache_ints_dictionary.xml']) + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + dictionary_node.query("create database if not exists test;") + dictionary_node.query("drop table if exists test.ints;") + dictionary_node.query("create table test.ints " + "(key UInt64, " + "i8 Int8, i16 Int16, i32 Int32, i64 Int64, " + "u8 UInt8, u16 UInt16, u32 UInt32, u64 UInt64) " + "Engine = Memory;") + dictionary_node.query("insert into test.ints values (7, 7, 7, 7, 7, 7, 7, 7, 7);") + dictionary_node.query("insert into test.ints values (5, 5, 5, 5, 5, 5, 5, 5, 5);") + + yield cluster + finally: + cluster.shutdown() + +# @pytest.mark.skip(reason="debugging") +def test_default_reading(started_cluster): + assert None != dictionary_node.get_process_pid("clickhouse"), "ClickHouse must be alive" + + # Key 0 is not in dictionary, so default value will be returned + + def test_helper(): + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'i8', toUInt64(13), toInt8(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'i16', toUInt64(13), toInt16(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'i32', toUInt64(13), toInt32(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'i64', toUInt64(13), toInt64(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'u8', toUInt64(13), toUInt8(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'u16', toUInt64(13), toUInt16(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'u32', toUInt64(13), toUInt32(42));").rstrip() + assert '42' == main_node.query("select dictGetOrDefault('anime_dict', 'u64', toUInt64(13), toUInt64(42));").rstrip() + + test_helper() + + with PartitionManager() as pm, ClickHouseKiller(dictionary_node): + assert None == dictionary_node.get_process_pid("clickhouse"), "CLickHouse must be alive" + + # Remove connection between main_node and dictionary for sure + pm.heal_all() + pm.partition_instances(main_node, dictionary_node) + + # Dictionary max lifetime is 2 seconds. + time.sleep(3) + + test_helper() + diff --git a/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test.py b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get.py similarity index 56% rename from dbms/tests/integration/test_dictionary_allow_read_expired_keys/test.py rename to dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get.py index e43b164b48394177b5561e98d8af4d7609418acf..6b0e19362592144bad8fa2135b940b068f35142e 100644 --- a/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test.py +++ b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get.py @@ -2,8 +2,10 @@ from __future__ import print_function import pytest import time import os +from contextlib import contextmanager from helpers.cluster import ClickHouseCluster +from helpers.cluster import ClickHouseKiller from helpers.network import PartitionManager from helpers.network import PartitionManagerDisabler @@ -12,7 +14,6 @@ cluster = ClickHouseCluster(__file__, base_configs_dir=os.path.join(SCRIPT_DIR, dictionary_node = cluster.add_instance('dictionary_node', stay_alive=True) main_node = cluster.add_instance('main_node', main_configs=['configs/dictionaries/cache_ints_dictionary.xml']) -pm = PartitionManager() @pytest.fixture(scope="module") def started_cluster(): @@ -20,40 +21,24 @@ def started_cluster(): cluster.start() dictionary_node.query("create database if not exists test;") dictionary_node.query("drop table if exists test.ints;") - dictionary_node.query("create table test.ints (key UInt64, i8 Int8, i16 Int16, i32 Int32, " - "i64 Int64, u8 UInt8, u16 UInt16, u32 UInt32, u64 UInt64) " + dictionary_node.query("create table test.ints " + "(key UInt64, " + "i8 Int8, i16 Int16, i32 Int32, i64 Int64, " + "u8 UInt8, u16 UInt16, u32 UInt32, u64 UInt64) " "Engine = Memory;") dictionary_node.query("insert into test.ints values (7, 7, 7, 7, 7, 7, 7, 7, 7);") - - print(dictionary_node.query("describe test.ints")) - print(dictionary_node.query("select * from test.ints")) + dictionary_node.query("insert into test.ints values (5, 5, 5, 5, 5, 5, 5, 5, 5);") yield cluster finally: cluster.shutdown() +# @pytest.mark.skip(reason="debugging") def test_simple_dict_get(started_cluster): + assert None != dictionary_node.get_process_pid("clickhouse"), "ClickHouse must be alive" - assert '7' == main_node.query("select dictGet('anime_dict', 'i8', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'i16', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'i32', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'i64', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'u8', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'u16', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'u32', toUInt64(7));").rstrip(), "Wrong answer." - assert '7' == main_node.query("select dictGet('anime_dict', 'u64', toUInt64(7));").rstrip(), "Wrong answer." - - with PartitionManager() as pm: - pm.partition_instances(main_node, dictionary_node, port=9000) - - # Kill the instance for sure - dictionary_node.kill_clickhouse() - assert None == dictionary_node.get_process_pid("clickhouse") - pm.heal_all() - pm.partition_instances(main_node, dictionary_node, port=9000) - - # print(dictionary_node.query("select * from test.ints")) + def test_helper(): assert '7' == main_node.query("select dictGet('anime_dict', 'i8', toUInt64(7));").rstrip(), "Wrong answer." assert '7' == main_node.query("select dictGet('anime_dict', 'i16', toUInt64(7));").rstrip(), "Wrong answer." assert '7' == main_node.query("select dictGet('anime_dict', 'i32', toUInt64(7));").rstrip(), "Wrong answer." @@ -62,3 +47,17 @@ def test_simple_dict_get(started_cluster): assert '7' == main_node.query("select dictGet('anime_dict', 'u16', toUInt64(7));").rstrip(), "Wrong answer." assert '7' == main_node.query("select dictGet('anime_dict', 'u32', toUInt64(7));").rstrip(), "Wrong answer." assert '7' == main_node.query("select dictGet('anime_dict', 'u64', toUInt64(7));").rstrip(), "Wrong answer." + + test_helper() + + with PartitionManager() as pm, ClickHouseKiller(dictionary_node): + assert None == dictionary_node.get_process_pid("clickhouse") + + # Remove connection between main_node and dictionary for sure + pm.heal_all() + pm.partition_instances(main_node, dictionary_node) + + # Dictionary max lifetime is 2 seconds. + time.sleep(3) + + test_helper() diff --git a/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get_or_default.py b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get_or_default.py new file mode 100644 index 0000000000000000000000000000000000000000..3fce7b7398d7e138e7f61d1e9696ef513b1f0399 --- /dev/null +++ b/dbms/tests/integration/test_dictionary_allow_read_expired_keys/test_dict_get_or_default.py @@ -0,0 +1,60 @@ +from __future__ import print_function +import pytest +import time +import os +from contextlib import contextmanager + +from helpers.cluster import ClickHouseCluster +from helpers.cluster import ClickHouseKiller +from helpers.network import PartitionManager + +SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) +cluster = ClickHouseCluster(__file__, base_configs_dir=os.path.join(SCRIPT_DIR, 'configs')) + +dictionary_node = cluster.add_instance('dictionary_node', stay_alive=True) +main_node = cluster.add_instance('main_node', main_configs=['configs/dictionaries/cache_ints_dictionary.xml']) + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + dictionary_node.query("create database if not exists test;") + dictionary_node.query("drop table if exists test.ints;") + dictionary_node.query("create table test.ints " + "(key UInt64, " + "i8 Int8, i16 Int16, i32 Int32, i64 Int64, " + "u8 UInt8, u16 UInt16, u32 UInt32, u64 UInt64) " + "Engine = Memory;") + dictionary_node.query("insert into test.ints values (7, 7, 7, 7, 7, 7, 7, 7, 7);") + dictionary_node.query("insert into test.ints values (5, 5, 5, 5, 5, 5, 5, 5, 5);") + + yield cluster + finally: + cluster.shutdown() + +# @pytest.mark.skip(reason="debugging") +def test_simple_dict_get_or_default(started_cluster): + assert None != dictionary_node.get_process_pid("clickhouse"), "ClickHouse must be alive" + + def test_helper(): + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'i8', toUInt64(5), toInt8(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'i16', toUInt64(5), toInt16(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'i32', toUInt64(5), toInt32(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'i64', toUInt64(5), toInt64(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'u8', toUInt64(5), toUInt8(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'u16', toUInt64(5), toUInt16(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'u32', toUInt64(5), toUInt32(42));").rstrip() + assert '5' == main_node.query("select dictGetOrDefault('anime_dict', 'u64', toUInt64(5), toUInt64(42));").rstrip() + + test_helper() + + with PartitionManager() as pm, ClickHouseKiller(dictionary_node): + assert None == dictionary_node.get_process_pid("clickhouse") + + # Remove connection between main_node and dictionary for sure + pm.partition_instances(main_node, dictionary_node) + + # Dictionary max lifetime is 2 seconds. + time.sleep(3) + + test_helper()