Skip to content

Commit 2539220

Browse files
committed
syn2mas: introduce a dry-run mode
1 parent 6d2e681 commit 2539220

File tree

2 files changed

+42
-10
lines changed

2 files changed

+42
-10
lines changed

crates/cli/src/commands/syn2mas.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,17 @@ enum Subcommand {
7070
///
7171
/// It is OK for Synapse to be online during these checks.
7272
Check,
73+
7374
/// Perform a migration. Synapse must be offline during this process.
74-
Migrate,
75+
Migrate {
76+
/// Perform a dry-run migration, which is safe to run with Synapse
77+
/// running, and will restore the MAS database to an empty state.
78+
///
79+
/// This still *does* write to the MAS database, making it more
80+
/// realistic compared to the final migration.
81+
#[clap(long)]
82+
dry_run: bool,
83+
},
7584
}
7685

7786
/// The number of parallel writing transactions active against the MAS database.
@@ -118,11 +127,10 @@ impl Options {
118127
.await
119128
.context("could not run migrations")?;
120129

121-
if matches!(&self.subcommand, Subcommand::Migrate) {
130+
if matches!(&self.subcommand, Subcommand::Migrate { .. }) {
122131
// First perform a config sync
123132
// This is crucial to ensure we register upstream OAuth providers
124133
// in the MAS database
125-
//
126134
let config = SyncConfig::extract(figment)?;
127135
let clock = SystemClock::default();
128136
let encrypter = config.secrets.encrypter();
@@ -201,7 +209,8 @@ impl Options {
201209

202210
Ok(ExitCode::SUCCESS)
203211
}
204-
Subcommand::Migrate => {
212+
213+
Subcommand::Migrate { dry_run } => {
205214
let provider_id_mappings: HashMap<String, Uuid> = {
206215
let mas_oauth2 = UpstreamOAuth2Config::extract_or_default(figment)?;
207216

@@ -217,8 +226,7 @@ impl Options {
217226

218227
// TODO how should we handle warnings at this stage?
219228

220-
// TODO this dry-run flag should be set to false in real circumstances !!!
221-
let reader = SynapseReader::new(&mut syn_conn, true).await?;
229+
let reader = SynapseReader::new(&mut syn_conn, dry_run).await?;
222230
let writer_mas_connections =
223231
futures_util::future::try_join_all((0..NUM_WRITER_CONNECTIONS).map(|_| {
224232
database_connection_from_config_with_options(
@@ -230,7 +238,8 @@ impl Options {
230238
}))
231239
.instrument(tracing::info_span!("syn2mas.mas_writer_connections"))
232240
.await?;
233-
let writer = MasWriter::new(mas_connection, writer_mas_connections).await?;
241+
let writer =
242+
MasWriter::new(mas_connection, writer_mas_connections, dry_run).await?;
234243

235244
let clock = SystemClock::default();
236245
// TODO is this rng ok?

crates/syn2mas/src/mas_writer/mod.rs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ impl FinishCheckerHandle {
242242
pub struct MasWriter {
243243
conn: LockedMasDatabase,
244244
writer_pool: WriterConnectionPool,
245+
dry_run: bool,
245246

246247
indices_to_restore: Vec<IndexDescription>,
247248
constraints_to_restore: Vec<ConstraintDescription>,
@@ -793,6 +794,7 @@ impl MasWriter {
793794
pub async fn new(
794795
mut conn: LockedMasDatabase,
795796
mut writer_connections: Vec<PgConnection>,
797+
dry_run: bool,
796798
) -> Result<Self, Error> {
797799
// Given that we don't have any concurrent transactions here,
798800
// the READ COMMITTED isolation level is sufficient.
@@ -902,7 +904,7 @@ impl MasWriter {
902904

903905
Ok(Self {
904906
conn,
905-
907+
dry_run,
906908
writer_pool: WriterConnectionPool::new(writer_connections),
907909
indices_to_restore,
908910
constraints_to_restore,
@@ -987,7 +989,6 @@ impl MasWriter {
987989

988990
// Now all the data has been migrated, finish off by restoring indices and
989991
// constraints!
990-
991992
query("BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;")
992993
.execute(self.conn.as_mut())
993994
.await
@@ -1009,6 +1010,28 @@ impl MasWriter {
10091010
.await
10101011
.into_database("could not revert temporary tables")?;
10111012

1013+
// If we're in dry-run mode, truncate all the tables we've written to
1014+
if self.dry_run {
1015+
warn!("Migration ran in dry-run mode, deleting all imported data");
1016+
let tables = MAS_TABLES_AFFECTED_BY_MIGRATION
1017+
.iter()
1018+
.map(|table| format!("\"{table}\""))
1019+
.collect::<Vec<_>>()
1020+
.join(", ");
1021+
1022+
// Note that we do that with CASCADE, because we do that *after*
1023+
// restoring the FK constraints.
1024+
//
1025+
// The alternative would be to list all the tables we have FK to
1026+
// those tables, which would be a hassle, or to do that after
1027+
// restoring the constraints, which would mean we wouldn't validate
1028+
// that we've done valid FKs in dry-run mode.
1029+
query(&format!("TRUNCATE TABLE {tables} CASCADE;"))
1030+
.execute(self.conn.as_mut())
1031+
.await
1032+
.into_database_with(|| "failed to truncate all tables")?;
1033+
}
1034+
10121035
query("COMMIT;")
10131036
.execute(self.conn.as_mut())
10141037
.await
@@ -1193,7 +1216,7 @@ mod test {
11931216
.await
11941217
.expect("failed to lock MAS database")
11951218
.expect_left("MAS database is already locked");
1196-
MasWriter::new(locked_main_conn, writer_conns)
1219+
MasWriter::new(locked_main_conn, writer_conns, false)
11971220
.await
11981221
.expect("failed to construct MasWriter")
11991222
}

0 commit comments

Comments
 (0)