Skip to content

Commit b77103c

Browse files
committed
fix dynamic_config: don't store defaults to cache
Changelog entry (fixes): * Dynamic config variables, for which the @ref dynamic_config_defaults "default value" is currently used, are no longer stored into the cache file. This fixes an interaction where the service would ignore a changed default value for a config variable. --- Let's look at a setup where the config service only sends configs which were explicitly overridden compared to the defaults. If 1. defaults change (e.g. by modifying `dynamic_config_fallback.json`), 2. the service restarts, and 3. there is an old config cache file then the cache will contain all configs, including the configs that the config service did not send, for which the defaults were used. The behavior will be as if the old defaults were somehow stuck, which is erroneous. commit_hash:68b4c2aa0ebca6d663cf636b7b4249b64f764cda
1 parent 63d05a9 commit b77103c

3 files changed

Lines changed: 24 additions & 3 deletions

File tree

core/functional_tests/dynamic_configs/tests/conftest.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@
55
pytest_plugins = ['pytest_userver.plugins.core']
66

77

8+
@pytest.fixture(name='_userver_config_dumps', scope='session')
9+
def _userver_config_dumps_fixture(_userver_config_dumps):
10+
def patch_config(config_yaml, config_vars) -> None:
11+
_userver_config_dumps(config_yaml, config_vars)
12+
config_vars['userver-dumps-periodic'] = True
13+
14+
return patch_config
15+
16+
817
# For test_fixtures.py
918
@pytest.fixture(scope='session')
1019
def dynamic_config_fallback_patch() -> dict[str, Any]:

core/functional_tests/dynamic_configs/tests/test_cache_updates.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
async def test_inc_update(service_client, testpoint):
1+
import json
2+
import pathlib
3+
4+
5+
def _load_dynamic_config_cache(service_config):
6+
cache_path = pathlib.Path(service_config['components_manager']['components']['dynamic-config']['fs-cache-path'])
7+
return json.loads(cache_path.read_text())
8+
9+
10+
async def test_inc_update(service_client, service_config, testpoint):
211
@testpoint('reset-cache-dynamic-config-client-updater')
312
def tp_reset(data):
413
pass
@@ -15,3 +24,6 @@ def tp_reset(data):
1524

1625
assert tp_reset.times_called == 1
1726
assert tp_reset.next_call() == {'data': {'update_type': 'full'}}
27+
28+
cache = _load_dynamic_config_cache(service_config)
29+
assert cache == {'HTTP_CLIENT_CONNECTION_POOL_SIZE': 777}

core/src/dynamic_config/storage/component.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,9 @@ void DynamicConfig::Impl::DoSetConfig(const dynamic_config::DocsMap& value) {
221221

222222
void DynamicConfig::Impl::SetConfig(std::string_view updater, dynamic_config::DocsMap&& value) {
223223
LOG_DEBUG() << "Setting new dynamic config value from '" << updater << "'";
224+
WriteFsCache(value);
224225
value.MergeMissing(fallback_config_);
225226
DoSetConfig(value);
226-
WriteFsCache(value);
227227
}
228228

229229
ComponentHealth DynamicConfig::Impl::GetComponentHealth() const {
@@ -327,7 +327,7 @@ void DynamicConfig::Impl::WriteFsCache(const dynamic_config::DocsMap& docs_map)
327327
fs::CreateDirectories(fs_task_processor_, boost::filesystem::path(fs_cache_path_).parent_path().string());
328328
fs::RewriteFileContentsAtomically(fs_task_processor_, fs_cache_path_, contents, mode);
329329

330-
LOG_INFO() << "Successfully wrote dynamic_config from FS cache";
330+
LOG_INFO() << "Successfully wrote dynamic_config to FS cache";
331331
} catch (const std::exception& e) {
332332
LOG_ERROR() << "Failed to save config to FS cache '" << fs_cache_path_ << "': " << e;
333333
}

0 commit comments

Comments
 (0)