Repository navigation
Allow separate configuration for the replica connection pooler - #3188
omkar619-dev wants to merge 6 commits into
Conversation
Signed-off-by: Omkar Shendge <omkarshendge619@gmail.com>
Signed-off-by: Omkar Shendge <omkarshendge619@gmail.com>
Signed-off-by: Omkar Shendge <omkarshendge619@gmail.com>
Signed-off-by: Omkar Shendge <omkarshendge619@gmail.com>
|
The End-2-End failure here doesn't look like it comes from this branch, though I'd welcome a correction. It fails in test_major_version_upgrade, on "AssertionError: Annotation for last upgrade's success is not updated". The same test also fails on master at b217b45, which is this PR's base: that run started about a minute before this This branch only touches the connection pooler, the CRD schema, deepcopy and the docs, so I can't see a route from it to a Postgres major version upgrade. If it would help, I'm happy to merge master in again to re-trigger the job once master is green — I'd rather do that than rebase, so the branch keeps the maintainer merge commits already in it. |
Adds a
replicaConnectionPoolersection to the Postgres manifest so the replica pooler can be configured independently of the master pooler.The new type carries
numberOfInstances,mode,dockerImage,maxDBConnectionsandresources. It deliberately does not carryuserorschema: those are cluster-scoped rather than per-pooler. The two places that read them —poolerUser, and the lookup-function install insyncConnectionPooler— create objects once per cluster that both poolers share, so leaving those two fields out means neither site has to become role-aware.Fields are pointer-typed so unset means "inherit from connectionPooler" rather than "zero". A manifest using
enableReplicaConnectionPooler: truewith only aconnectionPoolersection behaves exactly as it does today.How it works
effectiveConnectionPoolerSpec(spec, role)resolves the settings for a role: master returnsconnectionPoolerunchanged, replica returnsconnectionPoolerwith any field explicitly set inreplicaConnectionPoolerlaid over it. It returns the existingConnectionPoolertype, soneedSyncConnectionPoolerSpecsandneedSyncConnectionPoolerDefaultskeep their signatures, and it never returns nil, which replaces a nil check that four call sites each repeated.Those four are
getConnectionPoolerEnvVars(the only signature change — it gains the role, and has a single caller),generateConnectionPoolerPodTemplate(already took the role and ignored it),generateConnectionPoolerDeployment(has it via
connectionPooler.Role), and the spec comparison insyncConnectionPoolerWorker.That last one though is the subtle one. If the comparison keeps looking only at
spec.ConnectionPooler, editingreplicaConnectionPooleron a live cluster produces no diff, the deployment is never regenerated, and the replica poolersilently keeps its old settings — correct on create, inert on update.
TestReplicaConnectionPoolerSyncOnOverrideChangedrivessyncConnectionPooleragainst the fake client and asserts the replica deployment moves from 2 to 4instances while the master stays at 2. I confirmed it fails (expected 4, actual 2) with the role argument removed.
Also included
TestEffectiveConnectionPoolerSpec: six cases covering inherit, partial override, full override and the empty-section cases.replicaConnectionPoolersection in the manifest reference, an example in the user guide, and a commented block in the complete manifest.Left out deliberately
Setting
connectionPooleron its own implicitly enables the master pooler, but there is no replica equivalent, soenableReplicaConnectionPoolerremains required and areplicaConnectionPoolersection alone won't switch anything on.That felt outside the scope of this change, but happy to make the two symmetric if you'd prefer.
Closes #3113