From 41f0b33c7f4f3fb519cc29ab642e6c2d7837d353 Mon Sep 17 00:00:00 2001 From: Gabriel mermelstein Date: Fri, 17 Jan 2025 10:54:47 +0100 Subject: [PATCH 1/4] fix: avoid double db migration when both store and legacy store are mounted --- waku/factory/node_factory.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/waku/factory/node_factory.nim b/waku/factory/node_factory.nim index 80734d0b86..2318ca243a 100644 --- a/waku/factory/node_factory.nim +++ b/waku/factory/node_factory.nim @@ -267,8 +267,9 @@ proc setupProtocols( ## then the legacy will be in charge of performing the archiving. ## Regarding storage, the only diff between the current/future archive driver and the legacy ## one, is that the legacy stores an extra field: the id (message digest.) + let migrate = if conf.legacyStore: false else: conf.storeMessageDbMigration let archiveDriverRes = waitFor driver.ArchiveDriver.new( - conf.storeMessageDbUrl, conf.storeMessageDbVacuum, conf.storeMessageDbMigration, + conf.storeMessageDbUrl, conf.storeMessageDbVacuum, migrate, conf.storeMaxNumDbConnections, onFatalErrorAction, ) if archiveDriverRes.isErr(): From 40d12f9c170eb1b27a621772dda7fe4408168b03 Mon Sep 17 00:00:00 2001 From: Gabriel mermelstein Date: Fri, 17 Jan 2025 11:57:49 +0100 Subject: [PATCH 2/4] avoid second migration only for sqlite --- waku/factory/node_factory.nim | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/waku/factory/node_factory.nim b/waku/factory/node_factory.nim index 2318ca243a..4101519192 100644 --- a/waku/factory/node_factory.nim +++ b/waku/factory/node_factory.nim @@ -267,7 +267,13 @@ proc setupProtocols( ## then the legacy will be in charge of performing the archiving. ## Regarding storage, the only diff between the current/future archive driver and the legacy ## one, is that the legacy stores an extra field: the id (message digest.) - let migrate = if conf.legacyStore: false else: conf.storeMessageDbMigration + + var postgres = false + when defined(postgres): + postgres = true + + let migrate = + if not postgres and conf.legacyStore: false else: conf.storeMessageDbMigration let archiveDriverRes = waitFor driver.ArchiveDriver.new( conf.storeMessageDbUrl, conf.storeMessageDbVacuum, migrate, conf.storeMaxNumDbConnections, onFatalErrorAction, From 89d181a6481d495d161c92c405af3b97ae984716 Mon Sep 17 00:00:00 2001 From: Gabriel mermelstein Date: Fri, 17 Jan 2025 13:21:54 +0100 Subject: [PATCH 3/4] add comment --- waku/factory/node_factory.nim | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/waku/factory/node_factory.nim b/waku/factory/node_factory.nim index 4101519192..f815eec368 100644 --- a/waku/factory/node_factory.nim +++ b/waku/factory/node_factory.nim @@ -268,12 +268,18 @@ proc setupProtocols( ## Regarding storage, the only diff between the current/future archive driver and the legacy ## one, is that the legacy stores an extra field: the id (message digest.) + ## TODO: remove this "migrate" variable once legacy store is removed + ## It is now necessary because sqlite's legacy store has an extra field: storedAt + ## This breaks compatibility between store's and legacy store's schemas in sqlite + ## So for now, we need to make sure that when legacy store is enabled and we use sqlite + ## that we migrate our db according to legacy store's schema to have the extra field var postgres = false when defined(postgres): postgres = true let migrate = if not postgres and conf.legacyStore: false else: conf.storeMessageDbMigration + let archiveDriverRes = waitFor driver.ArchiveDriver.new( conf.storeMessageDbUrl, conf.storeMessageDbVacuum, migrate, conf.storeMaxNumDbConnections, onFatalErrorAction, From c160ef1f2a110fee4609c662e1bf346b18764e32 Mon Sep 17 00:00:00 2001 From: Gabriel mermelstein Date: Fri, 17 Jan 2025 16:14:41 +0100 Subject: [PATCH 4/4] checking dynamically the db type by url --- waku/factory/node_factory.nim | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/waku/factory/node_factory.nim b/waku/factory/node_factory.nim index f815eec368..d427a3d76b 100644 --- a/waku/factory/node_factory.nim +++ b/waku/factory/node_factory.nim @@ -35,7 +35,8 @@ import ../node/peer_manager/peer_store/migrations as peer_store_sqlite_migrations, ../waku_lightpush/common, ../common/utils/parse_size_units, - ../common/rate_limit/setting + ../common/rate_limit/setting, + ../common/databases/dburl ## Peer persistence @@ -273,12 +274,18 @@ proc setupProtocols( ## This breaks compatibility between store's and legacy store's schemas in sqlite ## So for now, we need to make sure that when legacy store is enabled and we use sqlite ## that we migrate our db according to legacy store's schema to have the extra field - var postgres = false - when defined(postgres): - postgres = true + + let engineRes = dburl.getDbEngine(conf.storeMessageDbUrl) + if engineRes.isErr(): + return err("error getting db engine in setupProtocols: " & engineRes.error) + + let engine = engineRes.get() let migrate = - if not postgres and conf.legacyStore: false else: conf.storeMessageDbMigration + if engine == "sqlite" and conf.legacyStore: + false + else: + conf.storeMessageDbMigration let archiveDriverRes = waitFor driver.ArchiveDriver.new( conf.storeMessageDbUrl, conf.storeMessageDbVacuum, migrate,