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.

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

add named volume to lnd service for persisting data accross container… #2533

Conversation

yancyribbens
Copy link
Contributor

Add a named volume to docker-compose.yml for persisting lnd application volume state. Without this volume, if the container is brought down (for example using docker-compose down) the state is lost, and lnd needs to resync, eg:

lnd_btc | 2019-01-18 16:38:41.154 [INF] LNWL: Caught up to height 10000

Adding this volume will persist the state along with btc similar to how a local install would work. The new volume can be inspected using docker volume inspect docker_lnd . Using a named volume in docker-compose allows docker to re-attach to the named volume during docker up. This is preferable to an anonymous volume which isn't recognized by docker-compose and so this PR replaces #2507

@halseth
Copy link
Contributor

halseth commented Jan 28, 2019

Does this also replace #697?

@yancyribbens
Copy link
Contributor Author

Yes unless you prefer to cherry pick from #697 instead. When I opened #2507 I wasn't aware of #697.

@halseth
Copy link
Contributor

halseth commented Jan 31, 2019

This is a much simpler change, so i would prefer sticking to this.

@yancyribbens
Copy link
Contributor Author

SGTM

@Roasbeef Roasbeef added enhancement Improvements to existing features / behaviour docker Docker-related PRs/Issues P3 might get fixed, nice to have needs review PR needs review by regular contributors labels Feb 1, 2019
Copy link
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

utACK 👍

@darwin
Copy link
Contributor

darwin commented Apr 12, 2019

This is definitely needed. I based my mainnet lnd setup on these docker scripts and did this change when tweaking it. But there is one additional subtle change you should make.

You should remove custom --logdir

--logdir="/data" \

By default, lnd logs into lnd's data folder /root/.lnd/logs/... which is what you want because that folder is mapped to host directory and persists. Logging into forced /data inside container would not persist logs between container rebuilds. Logs are not a big issue with simnet, but if someone took your setup (like I did) and based mainnet lnd setup on it, this could be an easy mistake.

@yancyribbens
Copy link
Contributor Author

Thanks @darwin. I've updated the PR to use the default home directory.

@halseth halseth requested a review from guggero November 7, 2019 10:22
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Sorry this has been lying around for so long. We'll be putting more focus on Docker stuff in the future.

There's a small nit in the comment.
And could you please also squash the two commits and a docker: prefix to the commit message?

docker/docker-compose.yml Outdated Show resolved Hide resolved
@guggero guggero self-assigned this Nov 7, 2019
@guggero
Copy link
Collaborator

guggero commented Nov 8, 2019

Thanks for the changes! I tested the tutorial again and it works as expected.

Something went wrong with the commits, now there are 5 instead of one. Feel free to ping me on Slack if you need help with git.

@yancyribbens
Copy link
Contributor Author

@guggero sure no problem! i'll rebase the commit history and ping you if needed.

@yancyribbens yancyribbens force-pushed the persistent-lnd-volume-docker-compose branch from dadfb61 to 5c4edca Compare November 8, 2019 17:33
@yancyribbens
Copy link
Contributor Author

@guggero comments have been squashed

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@halseth halseth merged commit 3b22540 into lightningnetwork:master Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Docker-related PRs/Issues enhancement Improvements to existing features / behaviour needs review PR needs review by regular contributors P3 might get fixed, nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants