Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvdb: add postgres #5366

Merged
merged 3 commits into from
Sep 21, 2021
Merged

kvdb: add postgres #5366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3eb80ca
kvdb: add postgres
joostjager Jul 13, 2021
ae6cf72
lncfg: add postgres
joostjager Jul 13, 2021
daeb96f
postgres: add itest
joostjager Jun 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ jobs:
matrix:
unit_type:
- btcd unit-cover
- unit tags=kvdb_etcd
- unit tags="kvdb_etcd kvdb_postgres"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you want separate lines for these tags to test them individually.

Copy link
Collaborator

@bhandras bhandras Jul 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Some more context: when we add the tag kvdb_etcd we also run channeldb unit tests on etcd.
The trick is that there's a GetTestBackend() function in kvdb/backend.go and there's a constant behind the tag in kvdb/kvdb_etcd.go and kvdb/kvdb_no_etcd.go that defines which backend to start with GetTestBacked().

Looking at it again I now think having the tags passed together works but it'll still run the channeldb tests on etcd. You may want to fix this such that those tests also run on Postgres.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran the channeldb tests on postgres and they all pass except for three specialty tests. Two fail because of assumptions about the db path (does not apply to postgres) and one because of panic bubbling that is different in Postgres. Needs a closer look.

My proposal is to do that work in a separate PR where we can also evaluate the additional run time for a test suite that is already long-running. For now, I at least have manual confirmation that the channeldb tests do pass on Postgres. Let me know your thoughts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Running the postgres tests as a separate suite sounds good to me. If that needs some more work because of special cases I agree it should be done in a separate PR. Knowing they pass (with just three explainable exceptions) is important though.

@joostjager can you create an issue please so we don't forget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed the code that I used here: #5550. I think this can double as an issue?

Copy link
Member

Choose a reason for hiding this comment

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

Isn't it the case that tacking on this extra tag doesn't actually do anything in practice? I'm guessing Go has some ordering tie-breaker here that causes it to only use the first tag?

In any case I think we want these to run separately so we can catch distinct regressions for both backends.

Copy link
Member

Choose a reason for hiding this comment

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

I ran the channeldb tests on postgres and they all pass except for three specialty tests.

Are these tests still failing?

Copy link
Contributor Author

@joostjager joostjager Aug 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

In any case I think we want these to run separately so we can catch distinct regressions for both backends.

They are running separately. There is kvdb/etcd_test.go and kvdb/postgres_test.go. Both build tags are set to have all code available in the test binary.

Are these tests still failing?

Still failing, I haven't continued the work on that PR #5550 above.

- travis-race
steps:
- name: git checkout
Expand Down
5 changes: 5 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ jobs:
- bash ./scripts/install_bitcoind.sh
- make itest-parallel backend=bitcoind dbbackend=etcd

- name: Bitcoind Integration with postgres (txindex enabled)
script:
- bash ./scripts/install_bitcoind.sh
- make itest-parallel backend=bitcoind dbbackend=postgres

- name: Bitcoind Integration (txindex disabled)
script:
- bash ./scripts/install_bitcoind.sh
Expand Down
17 changes: 15 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,20 @@ scratch: build

check: unit itest

itest-only:
db-instance:
ifeq ($(dbbackend),postgres)
# Remove a previous postgres instance if it exists.
docker rm lnd-postgres --force || echo "Starting new postgres container"

# Start a fresh postgres instance. Allow a maximum of 500 connections.
# This is required for the async benchmark to pass.
docker run --name lnd-postgres -e POSTGRES_PASSWORD=postgres -p 6432:5432 -d postgres -N 500

# Wait for the instance to be started.
sleep 3
endif

itest-only: db-instance
@$(call print, "Running integration tests with ${backend} backend.")
rm -rf lntest/itest/*.log lntest/itest/.logs-*; date
EXEC_SUFFIX=$(EXEC_SUFFIX) scripts/itest_part.sh 0 1 $(TEST_FLAGS) $(ITEST_FLAGS)
Expand All @@ -196,7 +209,7 @@ itest: build-itest itest-only

itest-race: build-itest-race itest-only

itest-parallel: build-itest
itest-parallel: build-itest db-instance
@$(call print, "Running tests")
rm -rf lntest/itest/*.log lntest/itest/.logs-*; date
EXEC_SUFFIX=$(EXEC_SUFFIX) echo "$$(seq 0 $$(expr $(ITEST_PARALLELISM) - 1))" | xargs -P $(ITEST_PARALLELISM) -n 1 -I {} scripts/itest_part.sh {} $(NUM_ITEST_TRANCHES) $(TEST_FLAGS) $(ITEST_FLAGS)
Expand Down
31 changes: 31 additions & 0 deletions docs/postgres.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Postgres support in LND

With the introduction of the `kvdb` interface, LND can support multiple database
backends. One of the supported backends is Postgres. This document
describes how it can be configured.

## Building LND with postgres support

To build LND with postgres support, include the following build tag:

```shell
⛰ make tags="kvdb_postgres"
```

## Configuring Postgres for LND

In order for LND to run on Postgres, an empty database should already exist. A
database can be created via the usual ways (psql, pgadmin, etc). A user with
access to this database is also required.

Creation of a schema and the tables is handled by LND automatically.

## Configuring LND for Postgres

LND is configured for Postgres through the following configuration options:

* `db.backend=postgres` to select the Postgres backend.
* `db.postgres.dsn=...` to set the database connection string that includes
database, user and password.
* `db.postgres.timeout=...` to set the connection timeout. If not set, no
timeout applies.
21 changes: 21 additions & 0 deletions docs/release-notes/release-notes-0.14.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,27 @@ instantaneous. Read the [guide on leader
election](https://github.com/lightningnetwork/lnd/blob/master/docs/leader_election.md)
for more information.

### Postgres database support

This release adds [support for Postgres as a database
backend](https://github.com/lightningnetwork/lnd/pull/5366) to lnd. Postgres
has several advantages over the default bbolt backend:
* Better handling of large data sets.
* On-the-fly database compaction (auto vacuum).
* Database replication.
* Inspect data while lnd is running (bbolt opens the database exclusively).
* Usage of industry-standard tools to manage the stored data, get performance
metrics, etc.

Furthermore, the SQL platform opens up possibilities to improve lnd's
performance in the future. Bbolt's single-writer model is a severe performance
bottleneck, whereas Postgres offers a variety of locking models. Additionally,
structured tables reduce the need for custom serialization/deserialization code
in `lnd`, saving developer time and limiting the potential for bugs.

Instructions for enabling Postgres can be found in
[docs/postgres.md](../postgres.md).

## Protocol Extensions

### Explicit Channel Negotiation
Expand Down
5 changes: 3 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/btcsuite/btcwallet/wallet/txrules v1.0.0
github.com/btcsuite/btcwallet/walletdb v1.3.6-0.20210803004036-eebed51155ec
github.com/btcsuite/btcwallet/wtxmgr v1.3.1-0.20210822222949-9b5a201c344c
github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e
github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f
github.com/davecgh/go-spew v1.1.1
github.com/fsnotify/fsnotify v1.4.9 // indirect
github.com/go-errors/errors v1.0.1
Expand All @@ -24,6 +24,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/grpc-ecosystem/grpc-gateway/v2 v2.5.0
github.com/jackc/pgx/v4 v4.13.0
github.com/jackpal/gateway v1.0.5
github.com/jackpal/go-nat-pmp v0.0.0-20170405195558-28a68d0c24ad
github.com/jedib0t/go-pretty v4.3.0+incompatible
Expand Down Expand Up @@ -55,7 +56,7 @@ require (
github.com/urfave/cli v1.20.0
go.etcd.io/etcd/client/pkg/v3 v3.5.0
go.etcd.io/etcd/client/v3 v3.5.0
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97
golang.org/x/net v0.0.0-20210913180222-943fd674d43e
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20210915083310-ed5796bab164 // indirect
Expand Down
Loading