Skip to content

Conversation

@pm47
Copy link
Member

@pm47 pm47 commented Mar 30, 2021

We still use sqlite as the primary db, but all calls are replicated
asynchronously on postgres.

The goal is to prepare a smooth transition from sqlite to postgres
on a production server. This is a very specific use case and most users
shouldn't use it, which is why the new config eclair.db.driver=dual is
not documented.

@pm47 pm47 changed the base branch from master to postgres-refactor March 30, 2021 13:14
@pm47 pm47 marked this pull request as draft March 30, 2021 13:15
Base automatically changed from postgres-refactor to master March 31, 2021 14:12
We still use sqlite as the primary db, but all calls are replicated
asynchronously on postgres.

The goal is to prepare a smooth transition from sqlite to postgres
on a production server. This is a very specific use case and most users
shouldn't use it, which is why the new config `eclair.db.driver=dual` is
not documented.
@pm47 pm47 marked this pull request as ready for review July 7, 2021 08:27
@pm47 pm47 requested a review from t-bast July 7, 2021 08:27
case Success(res) => res
case Failure(t) =>
logger.error("postgres error:\n", t)
throw t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to throw here?
Since Sqlite is our primary DB, shouldn't we just swallow errors since this dual mode is only meant to let us monitor how postgres behaves in practice? This way we're sure it doesn't impact the running node, and can be deployed on mainnet safely (hopefully?).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already works that way, because this is all within a future that we are not waiting on. We have to provide a return type or an exception, but the throw is effectively ignored.

@pm47 pm47 merged commit af8394a into master Jul 7, 2021
@pm47 pm47 deleted the dual-db branch July 7, 2021 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants