Skip to content

Conversation

@FxKu
Copy link
Member

@FxKu FxKu commented Jan 25, 2023

The current flow when adding streams in the manifest is as follows:

SNYC 1:

  1. [List Pods] syncStatefulSet compares desired (manifest) and effective Patroni config and restarts pods
  2. [List Pods] syncStreams (calling syncPostgresConfig) sets wal_level: logical which requires a Postgres restart only happening in syncStatefulSet, exit syncStreams early

SNYC 2:

  1. [List Pods] syncStatefulSet compares desired (manifest) and effective Patroni config and restarts pods
  2. [List Pods] syncStreams (calling syncPostgresConfig) sets wal_level: logical but config is already updated - no restart required anymore, proceed with publications and slots
  3. [List Pods] syncStreams (calling syncPostgresConfig) syncing replication slots in Patroni config

The PR proposes the following refactoring:

SNYC 1:

  1. [List Pods] syncStatefulSet sets wal_level: logical if streams are defined and then compares desired (manifest) and effective Patroni config (calling syncPatroniConfig) and restarts pods
  2. [List Pods] syncStreams expectes wal_level to be already configured and calls syncPatroniConfig to reflect the slots required for streams in Patroni config

It brings the following advantages:

  • 2 instead of 3 list pod calls to K8s API during each SYNC
  • stream CRD(s) will be created on the same UPDATE event when streams were added to the manifest - no need to wait for next SYNC
  • syncStatefulSet and syncStreams sharing the same code for syncing Postgres config
  • collect all error msg on config syncs and log them as a warning
  • less code in syncStatefulSet due to new functions makes the code easier to understand

@idanovinda
Copy link
Member

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Jan 25, 2023

👍

@FxKu FxKu added this to the 1.9 milestone Jan 25, 2023
@FxKu FxKu merged commit b916519 into master Jan 25, 2023
@FxKu FxKu deleted the restart-pg-to-create-streams branch January 25, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants