chore(store): improve database migrations logging

tests/v2/test_message_store_sqlite.nim

@@ -246,24 +246,6 @@ suite "Message Store":
     ## Cleanup
     store.close()
   
-  test "migration":
-    let 
-      database = SqliteDatabase.init("", inMemory = true)[]
-      store = SqliteStore.init(database)[]
-    defer: store.close()
-
-    template sourceDir: string = currentSourcePath.rsplit(DirSep, 1)[0]
-    let migrationPath = sourceDir
-
-    let res = database.migrate(migrationPath, 10)
-    check:
-      res.isErr == false
-
-    let ver = database.getUserVersion()
-    check:
-      ver.isErr == false
-      ver.value == 10
-
   # TODO: Move this test case to retention policy test suite
   test "number of messages retrieved by getAll is bounded by storeCapacity":
     let capacity = 10

waku/v2/node/storage/migration.nim

@@ -0,0 +1,81 @@
+
+import
+  stew/results,
+  chronicles
+import
+  ./sqlite,
+  ./migration/migration_types,
+  ./migration/migration_utils
+
+export
+  migration_types,
+  migration_utils
+
+
+logScope:
+  topics = "storage.migration"
+
+
+const USER_VERSION* = 7 # increase this when there is an update in the database schema
+
+
+proc migrate*(db: SqliteDatabase, path: string, targetVersion: int64 = USER_VERSION): DatabaseResult[void] = 
+  ## Compares the user_version of the db with the targetVersion 
+  ## runs migration scripts if the user_version is outdated (does not support down migration)
+  ## path points to the directory holding the migrations scripts
+  ## once the db is updated, it sets the user_version to the tragetVersion
+  
+  # read database version
+  let userVersionRes = db.getUserVersion()
+  if userVersionRes.isErr():
+    debug "failed to get user_version", error=userVersionRes.error
+
+  let userVersion = userVersionRes.value
+
+  debug "current database user_version", userVersion=userVersion, targetVersion=targetVersion
+
+  if userVersion == targetVersion:
+    info "database is up to date"
+    return ok()
+
+  info "database user_version outdated. migrating.", userVersion=userVersion, targetVersion=targetVersion
+
+  # fetch migration scripts
+  let migrationScriptsRes = getScripts(path)
+  if migrationScriptsRes.isErr():
+    return err("failed to load migration scripts")
+
+  let migrationScripts = migrationScriptsRes.value
+
+  # filter scripts based on their versions
+  let scriptsRes = migrationScripts.filterScripts(userVersion, targetVersion)
+  if scriptsRes.isErr():
+    return err("failed to filter migration scripts")
+  
+  let scripts = scriptsRes.value
+  if scripts.len == 0:
+    return err("no suitable migration scripts")
+  
+  trace "migration scripts", scripts=scripts
+  
+  
+  # Run the migration scripts
+  for script in scripts:
+
+    for query in script.splitScript():
+      debug "executing migration statement", statement=query
+
+      let execRes = db.query(query, NoopRowHandler)
+      if execRes.isErr():
+        error "failed to execute migration statement", statement=query, error=execRes.error
+        return err("failed to execute migration statement")
+
+      debug "migration statement executed succesfully", statement=query
+    
+  # Update user_version
+  let res = db.setUserVersion(targetVersion)
+  if res.isErr():
+    return err("failed to set the new user_version")
+
+  debug "database user_version updated", userVersion=targetVersion
+  ok()

waku/v2/node/storage/migration/migration_types.nim

@@ -7,8 +7,6 @@ const MESSAGE_STORE_MIGRATION_PATH* = sourceDir / "migrations_scripts/message"
 const PEER_STORE_MIGRATION_PATH* = sourceDir / "migrations_scripts/peer"
 const ALL_STORE_MIGRATION_PATH* = sourceDir / "migrations_scripts"
 
-const USER_VERSION* = 7 # increase this when there is an update in the database schema
-
 type MigrationScriptsResult*[T] = Result[T, string]
 type
   MigrationScripts* = ref object of RootObj

waku/v2/node/storage/migration/migration_utils.nim

@@ -9,7 +9,8 @@ import
 export migration_types
 
 logScope:
-  topics = "migration_utils"
+  topics = "storage.migration"
+
 
 proc getScripts*(migrationPath: string): MigrationScriptsResult[MigrationScripts] =
   ## the code in this procedure is an adaptation of https://github.com/status-im/nim-status/blob/21aebe41be03cb6450ea261793b800ed7d3e6cda/nim_status/migrations/sql_generate.nim#L4

waku/v2/node/storage/sqlite.nim

@@ -1,11 +1,11 @@
 {.push raises: [Defect].}
 
 import 
-  os,
-  sqlite3_abi,
-  chronicles,
+  std/os,
   stew/results,
-  migration/migration_utils
+  chronicles,
+  sqlite3_abi
+  
 # The code in this file is an adaptation of the Sqlite KV Store found in nim-eth.
 # https://github.com/status-im/nim-eth/blob/master/eth/db/kvstore_sqlite3.nim
 #
@@ -323,9 +323,11 @@ proc getUserVersion*(database: SqliteDatabase): DatabaseResult[int64] =
   var version: int64
   proc handler(s: ptr sqlite3_stmt) = 
     version = sqlite3_column_int64(s, 0)
+
   let res = database.query("PRAGMA user_version;", handler)
-  if res.isErr:
+  if res.isErr():
       return err("failed to get user_version")
+
   ok(version)
 
 
@@ -334,73 +336,10 @@ proc setUserVersion*(database: SqliteDatabase, version: int64): DatabaseResult[v
   ## some context borrowed from https://www.sqlite.org/pragma.html#pragma_user_version
   ## The user-version is an integer that is available to applications to use however they want. 
   ## SQLite makes no use of the user-version itself
-  proc handler(s: ptr sqlite3_stmt) = discard
-
   let query = "PRAGMA user_version=" & $version & ";"
-  let res = database.query(query, handler)
+  let res = database.query(query, NoopRowHandler)
   if res.isErr():
       return err("failed to set user_version")
 
   ok()
 
-
-proc migrate*(db: SqliteDatabase, path: string, targetVersion: int64 = migration_utils.USER_VERSION): DatabaseResult[bool] = 
-  ## compares the user_version of the db with the targetVersion 
-  ## runs migration scripts if the user_version is outdated (does not support down migration)
-  ## path points to the directory holding the migrations scripts
-  ## once the db is updated, it sets the user_version to the tragetVersion
-  
-  # read database version
-  let userVersion = db.getUserVersion()
-  debug "current db user_version", userVersion=userVersion
-  if userVersion.value == targetVersion:
-    # already up to date
-    info "database is up to date"
-    ok(true)
-  
-  else:
-    info "database user_version outdated. migrating.", userVersion=userVersion, targetVersion=targetVersion
-    # TODO check for the down migrations i.e., userVersion.value > tragetVersion
-    # fetch migration scripts
-    let migrationScriptsRes = getScripts(path)
-    if migrationScriptsRes.isErr:
-      return err("failed to load migration scripts")
-    let migrationScripts = migrationScriptsRes.value
-  
-    # filter scripts based on their versions
-    let scriptsRes = migrationScripts.filterScripts(userVersion.value, targetVersion)
-    if scriptsRes.isErr:
-      return err("failed to filter migration scripts")
-    
-    let scripts = scriptsRes.value
-    if (scripts.len == 0):
-      return err("no suitable migration scripts")
-    
-    debug "scripts to be run", scripts=scripts
-    
-    
-    proc handler(s: ptr sqlite3_stmt) = 
-      discard
-    
-    # run the scripts
-    for script in scripts:
-      debug "script", script=script
-      # a script may contain multiple queries
-      let queries = script.splitScript()
-      # TODO queries of the same script should be executed in an atomic manner
-      for query in queries:
-        let res = db.query(query, handler)
-        if res.isErr:
-          debug "failed to run the query", query=query
-          return err("failed to run the script")
-        else:
-          debug "query is executed", query=query
-
-    
-    # bump the user version
-    let res = db.setUserVersion(targetVersion)
-    if res.isErr:
-      return err("failed to set the new user_version")
-
-    debug "user_version is set to", targetVersion=targetVersion
-    ok(true)

waku/v2/node/wakunode2_setup_sql_migrations.nim

@@ -4,7 +4,7 @@ import
   stew/results,
   chronicles,
   ./storage/sqlite,
-  ./storage/migration/migration_types,
+  ./storage/migration,
   ./config
 
 logScope:
@@ -15,15 +15,14 @@ proc runMigrations*(sqliteDatabase: SqliteDatabase, conf: WakuNodeConf) =
   # Run migration scripts on persistent storage
   var migrationPath: string
   if conf.persistPeers and conf.persistMessages:
-    migrationPath = migration_types.ALL_STORE_MIGRATION_PATH
+    migrationPath = ALL_STORE_MIGRATION_PATH
   elif conf.persistPeers:
-    migrationPath = migration_types.PEER_STORE_MIGRATION_PATH
+    migrationPath = PEER_STORE_MIGRATION_PATH
   elif conf.persistMessages:
-    migrationPath = migration_types.MESSAGE_STORE_MIGRATION_PATH
+    migrationPath = MESSAGE_STORE_MIGRATION_PATH
 
-  info "running migration ...", migrationPath=migrationPath
   let migrationResult = sqliteDatabase.migrate(migrationPath)
   if migrationResult.isErr():
-    warn "migration failed", error=migrationResult.error()
+    warn "migration failed", error=migrationResult.error
   else:
     info "migration is done"