WIP: Adding hikari connection pool, release db connections back to pool af…

[Christewart]

…ter finished executing a statement, add hikari configuration stuff to reference.conf

This PR adds a connection pool to eclair so it is easier to manage database connections. I chose HikariCP as the connection pool which is used by scala slick.

There is hikari connection pool configuration stuff in the reference.conf file now, and we can add things later if we want to adjust our pool settings. See the home page of hikari for information about configuration. Hopefully this will also make it easier for eclair developers to use different databases than sqlite (we probably will use Postgres at SuredBits).

The last thing that is different about this PR is we are now closing the statement and the connection in the using util method. This will release the jdbc connection back to hikari.

[Christewart]

This also adds a new database, called unittest that is intended for unit testing eclair. It appears that this database isn't currently initialized correctly which is causing issues.

[araspitzu]

This might cause troubles for other users than "chris". You could replace it with ~ but i'm afraid it might break the windows builds. An option is to use environment variables as specified here and use {$HOME} instead of ~ (still windows builds might be affected).

[Christewart]

Most definitely 👍 I'm going to add the WIP in tag.

[pm47]

I'm not a fan of exposing the DbManagement/DbConfig in the db traits, to me connection pools are implementation details. For example some drivers such as postgres-async include connection pools.

Instead you could do a jdbc-based implementation of the db traits (e.g. JdbcChannelsDb and so on), that would encapsulate all that?

As for sqlite, I'm not sure whether we should leave the current implementation as-is (it is simple and don't need connection pool anyway), or if we should just use it through the standard jdbc (but it always gets messy with non-portable statements like "INSERT OR IGNORE" anyway).

[Christewart]

@pm47 I agree with your comment about connection pools being a implementation detail, but that is the only implementation now. I think my abstraction in DbConfig would work for another driver that supports connection pooling?

From what I understand the bigger problem you have is having the DbConfig taking in a HikariConfig as a constructor? I can try to move that into the companion object and hide that implementation detail and force people to go through the EclairDbConfig/NetworkDbConfig companion objects to get an instance of DbConfig.

[pm47]

I think my abstraction in DbConfig would work for another driver that supports connection pooling?

It only works with jdbc drivers, which are blocking. Not all drivers implement java.sql.* e.g. postgres-async.

From what I understand the bigger problem you have is having the DbConfig taking in a HikariConfig as a constructor?

We have tried to keep the *Db.scala traits as minimal and independant from implementation/models as possible: I would like to keep them free from concepts such as connections and table. Not all of those traits share the same characteristics: some are better implemented with a relational model (e.g. network db), but others could be stored in a no-sql db (e.g. channels). Some are better managed locally (e.g. network db), some could be put in an external server (e.g. channels or payments).

To be clear, instead of having:

package db {
  trait DbManagement { ... }
  trait NetworkDb extends DbManagement { ... }
}
package sqlite {
  class SqliteNetworkDb(override val dbConfig: NetworkDbConfig) extends NetworkDb { ... }
}

I'd prefer:

package db {
  trait NetworkDb { ... }
}
package sqlite {
  trait DbManagement { ... }
  class SqliteNetworkDb extends NetworkDb with DbManagement { ... }
}

With maybe s/sqlite/jdbc/ if we can iron out non-portable statements.

[t-bast]

@Christewart is that old PR still relevant? Or should we close this?