Skip to content

Commit 755dc76

Browse files
authored
feat(cli) - add replication role and replica identity checks (#489)
* Add replication role and replica identity checks * Fix to show all DB results at once and formatted some msgs. * Show the per DB results in sorted order.
1 parent bc589ea commit 755dc76

File tree

5 files changed

+111
-5
lines changed

5 files changed

+111
-5
lines changed

src/dma/collector/query_managers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ def get_collection_queries(self) -> set[str]:
284284
"collection_postgres_schema_objects",
285285
"collection_postgres_settings",
286286
"collection_postgres_source_details",
287+
"collection_postgres_replication_role",
287288
}
288289

289290
def get_per_db_collection_queries(self) -> set[str]:
@@ -301,6 +302,7 @@ def get_per_db_collection_queries(self) -> set[str]:
301302
"collection_postgres_user_views_without_privilege",
302303
"collection_postgres_user_sequences_without_privilege",
303304
"collection_postgres_tables_with_no_primary_key",
305+
"collection_postgres_tables_with_primary_key_replica_identity",
304306
}
305307

306308
def get_collection_filenames(self) -> dict[str, str]:
@@ -329,6 +331,8 @@ def get_collection_filenames(self) -> dict[str, str]:
329331
"collection_postgres_source_details": "postgres_source_details",
330332
"collection_postgres_pglogical_provider_node": "postgres_pglogical_details",
331333
"collection_postgres_tables_with_no_primary_key": "postgres_table_details",
334+
"collection_postgres_tables_with_primary_key_replica_identity": "postgres_table_details",
335+
"collection_postgres_replication_role": "collection_privileges",
332336
}
333337

334338

src/dma/collector/sql/canonical/ddls-postgres-collection.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,24 @@ create or replace table collection_postgres_tables_with_no_primary_key(
885885
database_name VARCHAR
886886
);
887887

888+
create or replace table collection_postgres_tables_with_primary_key_replica_identity(
889+
pkey VARCHAR,
890+
dma_source_id VARCHAR,
891+
dma_manual_id VARCHAR,
892+
nspname VARCHAR,
893+
relname VARCHAR,
894+
database_name VARCHAR
895+
);
896+
897+
create or replace table collection_postgres_replication_role(
898+
pkey VARCHAR,
899+
dma_source_id VARCHAR,
900+
dma_manual_id VARCHAR,
901+
rolname VARCHAR,
902+
rolreplication VARCHAR,
903+
database_name VARCHAR
904+
);
905+
888906
create or replace table collection_postgres_pglogical_provider_node(
889907
pkey VARCHAR,
890908
dma_source_id VARCHAR,

src/dma/collector/sql/sources/postgres/collection-privileges.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,16 @@ select :PKEY as pkey,
137137
src.relname as rel_name,
138138
current_database() as database_name
139139
from src;
140+
141+
-- name: collection-postgres-replication-role
142+
with src as (
143+
SELECT rolname, rolreplication FROM pg_catalog.pg_roles
144+
WHERE rolname IN (SELECT CURRENT_USER)
145+
)
146+
select :PKEY as pkey,
147+
:DMA_SOURCE_ID as dma_source_id,
148+
:DMA_MANUAL_ID as dma_manual_id,
149+
src.rolname,
150+
src.rolreplication,
151+
current_database() as database_name
152+
from src;

src/dma/collector/sql/sources/postgres/collection-table_details.sql

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,3 +467,30 @@ select :PKEY as pkey,
467467
src.relname,
468468
current_database() as database_name
469469
from src;
470+
471+
-- name: collection-postgres-tables-with-primary-key-replica-identity
472+
with src as (
473+
SELECT nr.nspname, r.relname
474+
FROM pg_namespace nr,
475+
pg_class r,
476+
pg_namespace nc,
477+
pg_constraint c
478+
WHERE
479+
nr.oid = r.relnamespace
480+
AND r.oid = c.conrelid
481+
AND nc.oid = c.connamespace
482+
AND c.contype = 'p'::"char"
483+
AND r.relkind = 'r'::"char"
484+
AND r.relpersistence = 'p'::"char"
485+
AND r.relreplident IN ('f'::"char", 'n'::"char")
486+
AND NOT pg_catalog.pg_is_other_temp_schema(nr.oid)
487+
AND nr.nspname not in ('pg_catalog', 'information_schema', 'pglogical', 'pglogical_origin')
488+
and nr.nspname not like 'pg\_%%'
489+
)
490+
select :PKEY as pkey,
491+
:DMA_SOURCE_ID as dma_source_id,
492+
:DMA_MANUAL_ID as dma_manual_id,
493+
src.nspname,
494+
src.relname,
495+
current_database() as database_name
496+
from src;

src/dma/collector/workflows/readiness_check/_postgres/main.py

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
PRIVILEGES: Final = "PRIVILEGES"
5050
PGLOGICAL_NODE_ALREADY_EXISTS: Final = "PGLOGICAL_NODE_ALREADY_EXISTS"
5151
TABLES_WITH_NO_PK: Final = "TABLES_WITH_NO_PK"
52+
UNSUPPORTED_TABLES_WITH_REPLICA_IDENTITY: Final = "UNSUPPORTED_TABLES_WITH_REPLICA_IDENTITY"
53+
REPLICATION_ROLE: Final = "REPLICATION_ROLE"
5254
CLOUDSQL: Final = "CLOUDSQL"
5355
ALLOYDB: Final = "ALLOYDB"
5456
CloudSQL_SUPER_ROLE: Final = "cloudsqladmin"
@@ -126,21 +128,23 @@ def execute(self) -> None:
126128
self._check_max_replication_slots()
127129
self._check_max_wal_senders_replication_slots()
128130
self._check_max_worker_processes()
129-
self._check_extensions()
130131
self._check_fdw()
131132

132133
# Per DB Checks.
134+
self._check_extensions()
135+
self._check_replication_role()
133136
# db_check_results stores the verification results for all DBs.
134137
for config in self.rule_config:
135138
db_check_results: dict[str, dict[str, list]] = {}
136-
for db in self.get_all_dbs():
139+
for db in sorted(self.get_all_dbs()):
137140
is_pglogical_installed = self._check_pglogical_installed(db, db_check_results)
138141
if is_pglogical_installed:
139142
privilege_check_passed = self._check_privileges(db, db_check_results)
140143
if not privilege_check_passed:
141-
break
144+
continue
142145
self._check_if_node_exists(db, db_check_results)
143146
self._check_tables_without_pk(db, db_check_results)
147+
self._check_tables_replica_identity(db, db_check_results)
144148
self._save_results(config.db_variant, db_check_results)
145149

146150
def _save_results(self, db_variant: PostgresVariants, db_check_results: dict[str, dict[str, list]]) -> None:
@@ -157,9 +161,9 @@ def _save_results(self, db_variant: PostgresVariants, db_check_results: dict[str
157161

158162
if rule == PRIVILEGES:
159163
if severity == ACTION_REQUIRED:
160-
output_str = ";".join(result[ACTION_REQUIRED])
164+
output_str = ";\n\n".join(result[ACTION_REQUIRED])
161165
elif severity == PASS:
162-
output_str = ";".join(result[PASS])
166+
output_str = ";\n".join(result[PASS])
163167
else:
164168
continue
165169

@@ -179,6 +183,12 @@ def _save_results(self, db_variant: PostgresVariants, db_check_results: dict[str
179183
else:
180184
continue
181185

186+
if rule == UNSUPPORTED_TABLES_WITH_REPLICA_IDENTITY:
187+
if severity == ACTION_REQUIRED:
188+
output_str = f"Source has table(s) with both primary key and replica identity FULL or NOTHING. Please remove replica identity or change it to DEFAULT to migrate: {';'.join(result[ACTION_REQUIRED])}"
189+
else:
190+
continue
191+
182192
if len(result[severity]) > 0:
183193
self.save_rule_result(
184194
db_variant,
@@ -225,6 +235,17 @@ def _check_tables_without_pk(self, db_name: str, db_check_results: dict[str, dic
225235
if tables:
226236
db_check_results[rule_code][WARNING].append(f"In database {db_name}, {tables} don't have primary keys")
227237

238+
def _check_tables_replica_identity(self, db_name: str, db_check_results: dict[str, dict[str, list]]) -> None:
239+
rule_code = UNSUPPORTED_TABLES_WITH_REPLICA_IDENTITY
240+
result = self.local_db.sql(
241+
"select CONCAT(nspname, '.', relname) from collection_postgres_tables_with_primary_key_replica_identity where database_name = $db_name",
242+
params={"db_name": db_name},
243+
).fetchall()
244+
tables = ", ".join(row[0] for row in result)
245+
init_results_dict(db_check_results, rule_code)
246+
if tables:
247+
db_check_results[rule_code][ACTION_REQUIRED].append(f"{tables} in database {db_name}")
248+
228249
def _check_version(self) -> None:
229250
rule_code = "DATABASE_VERSION"
230251
self.console.print(f"version: {self.db_version}")
@@ -357,6 +378,29 @@ def check_user_obj_privileges(self, db_name: str) -> list[str]:
357378
errors.extend(f"user doesn't have SELECT privilege on sequence {row[0]}.{row[1]}" for row in rows)
358379
return errors
359380

381+
def _check_replication_role(self) -> None:
382+
if self._is_rds():
383+
return
384+
rule_code = REPLICATION_ROLE
385+
result = self.local_db.sql("SELECT rolreplication FROM collection_postgres_replication_role").fetchone()
386+
if result is None:
387+
return
388+
for c in self.rule_config:
389+
if result[0] == "false":
390+
self.save_rule_result(
391+
c.db_variant,
392+
rule_code,
393+
ACTION_REQUIRED,
394+
"user does not have rolreplication role.",
395+
)
396+
else:
397+
self.save_rule_result(
398+
c.db_variant,
399+
rule_code,
400+
PASS,
401+
"user has rolreplication role.",
402+
)
403+
360404
def _check_privileges(self, db_name: str, db_check_results: dict[str, dict[str, list]]) -> bool:
361405
rule_code = PRIVILEGES
362406
errors = self._check_pglogical_privileges(db_name)

0 commit comments

Comments
 (0)