canonical / canonical/postgresql-single-kernel-library
Config render performs a redundant goal-state round-trip per update_config; dedupe with the caller's read or memoize per hook invocation
- Dominant language
- Python
- Stars
- 4
- Forks
- 3
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 26
Description
## Steps to reproduce
1. Deploy a PostgreSQL charm (VM or K8s) whose charm code calls `ConfigManager.update_config()` — this runs on every `update-status` and on every config-affecting hook.
2. Observe the hook-tool traffic of a single hook invocation (DEBUG log or `juju debug-log --replay`): within one `update_config()` call, the `goal-state` hook command is executed **twice** —
- once by the charm itself (membership/lag checks such as `get_running_cluster_members() < planned_units`),
- once by the lib's render path: `managers/config.py update_config` → `render_patroni_yml_file` → `core/state.py synchronous_node_count` (cached_property per state object) → `core/peer_relation.py planned_units` (cached_property per state object) → `ops Application.planned_units()` — an **uncached** `goal-state` hook command (verified in ops 3.8.1 and 3.8.2, `ops/model.py _canonicalize_id`-adjacent `planned_units` implementation).
The two `cached_property`s dedupe only *within* a single render; the charm's own read is a separate, second round-trip in the same hook.
## Expected behavior
One `goal-state` read per hook invocation, serving both the charm's own reads and the lib's config render. ops' documented contract states the data is "up-to-date as of the start of the current hook invocation", so reads are dedupable within a hook.
Suggested options (either):
- accept the already-read value from the caller: `update_config(..., planned_units: int | None = None)` (or an equivalent state-input parameter), so the charm shares its single read with the render; or
- memoize `planned_units` per hook invocation inside the lib (safe within a hook by the snapshot semantics above).
## Actual behavior
Two `goal-state` executions per `update_config()` call — on every `update-status` and every config-affecting hook, for every unit of every charm built on this library (VM and K8s). The charm cannot avoid its half: `update_config` accepts no `planned_units` parameter (verified in 16.3.8), and a charm-side memoization cannot reach the lib's render.
Constraint for any fix: **do not cache across hook invocations.** `planned_units` includes pending units and changes on scale events, and freshness per hook is a relied-upon contract (e.g. the restore pre-checks read it fresh per action, codified in the charm's unit tests). On K8s the charm process is long-lived, so any cache must be keyed to the render/hook, not to a long-lived object.
## Versions
Operating system: Ubuntu 24.04 (observed on an LXD VM backend; the code path is substrate-independent)
Juju CLI: 3.6.28
Juju agent: 3.6.28
Charm revision: canonical/postgresql-operator `dpe-10203-async-replication-recovery` / `dpe-10203-dead-dc-recovery-test` (local builds, revision 0) and the `16/edge` published base charm; library verified at `postgresql-charms-single-kernel` 16.3.8 with ops 3.8.1/3.8.2 (same behavior)
LXD: 5.21
microk8s: not used (VM substrate observed; the path is shared with the K8s charm)
## Log output
Juju debug log:
The read chain as observed in a live reproduction (DPE-10203 dead-DC recovery work):
```
charm.update_config (charm.py)
-> lib config.py update_config
-> config.py render_patroni_yml_file
-> state.py synchronous_node_count [cached_property per state object]
-> peer_relation.py planned_units [cached_property per state object]
-> ops Application.planned_units() -> goal-state [uncached]
```
plus the charm's own separate `planned_units()` read in the same hook.
## Additional context
- Companion issue #285 covers the resilience side of the same read (guard `planned_units()` against `ModelError` when a force-removed cross-model SAAS lingers in goal-state). This issue is the efficiency side: don't perform the read twice per hook. Cross-referencing both.
- Observed during review of canonical/postgresql-operator#1913 (comment thread on `_planned_units`/goal-state cost).
- Note for any fix: a charm-side cache alone does not help (the charm reads once per hook), and caching across hooks would be incorrect — hence the ask landing here.
Contributor guide
No contributing guide indexed for this repository
Research direction
Trace managers/config.py update_config through render_patroni_yml_file, core/state.py synchronous_node_count, and core/peer_relation.py planned_units, then compare it with the charm's own planned_units read in charm.py. Reproduce the hook-tool traffic with debug logging and verify that the fix performs one goal-state read per hook invocation, while preserving fresh values across hook invocations and scale events.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, python
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100