Skip to content

Commit

Permalink
tests & benchmarks: unify the way we customize the default tenant con…
Browse files Browse the repository at this point in the history
…fig (#9992)

Before this PR, some override callbacks used `.default()`, others
used `.setdefault()`.

As of this PR, all callbacks use `.setdefault()` which I think is least
prone to failure.

Aligning on a single way will set the right example for future tests
that need such customization.

The `test_pageserver_getpage_throttle.py` technically is a change in
behavior: before, it replaced the `tenant_config` field, now it just
configures the throttle. This is what I believe is intended anyway.
  • Loading branch information
problame authored Dec 3, 2024
1 parent ca85f36 commit 944c1ad
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 5 deletions.
3 changes: 1 addition & 2 deletions test_runner/performance/test_branch_creation.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ def test_branch_creation_many(neon_compare: NeonCompare, n_branches: int, shape:
# start without gc so we can time compaction with less noise; use shorter
# period for compaction so it starts earlier
def patch_default_tenant_config(config):
tenant_config = config.get("tenant_config", {})
tenant_config = config.setdefault("tenant_config", {})
tenant_config["compaction_period"] = "3s"
tenant_config["gc_period"] = "0s"
config["tenant_config"] = tenant_config

env.pageserver.edit_config_toml(patch_default_tenant_config)
env.pageserver.start(
Expand Down
3 changes: 1 addition & 2 deletions test_runner/regress/test_disk_usage_eviction.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ def assert_overrides(tenant_id, default_tenant_conf_value):
if config_level_override is not None:

def set_min_resident_size(config):
tenant_config = config.get("tenant_config", {})
tenant_config = config.setdefault("tenant_config", {})
tenant_config["min_resident_size_override"] = config_level_override
config["tenant_config"] = tenant_config

env.pageserver.edit_config_toml(set_min_resident_size)
env.pageserver.stop()
Expand Down
3 changes: 2 additions & 1 deletion test_runner/regress/test_pageserver_getpage_throttle.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ def test_throttle_fair_config_is_settable_but_ignored_in_config_toml(
"""

def set_tenant_config(ps_cfg):
ps_cfg["tenant_config"] = {"timeline_get_throttle": throttle_config_with_field_fair_set}
tenant_config = ps_cfg.setdefault("tenant_config", {})
tenant_config["timeline_get_throttle"] = throttle_config_with_field_fair_set

neon_env_builder.pageserver_config_override = set_tenant_config
env = neon_env_builder.init_start()
Expand Down

1 comment on commit 944c1ad

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7155 tests run: 6837 passed, 0 failed, 318 skipped (full report)


Flaky tests (9)

Postgres 17

Postgres 15

Postgres 14

Code coverage* (full report)

  • functions: 30.8% (8265 of 26844 functions)
  • lines: 47.8% (65171 of 136375 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
944c1ad at 2024-12-03T23:59:10.450Z :recycle:

Please sign in to comment.