feat: add backup daemon #1032

michael1011 · 2019-06-14T14:25:49Z

Closes #917

This PR adds xu-backup that starts listening for new LND SCBs and writing them to the disk when executed.

I am not finished yet and have some things to do before this backup daemon can be used but please have a look at this PR nevertheless to acknowledge the concept and give me some feedback on what I have been doing in this branch.

Todo:

Raiden database backup (just copying the file is the recommended way of the Raiden devs)
XUD database backup
write SCB of all LNDs and Raiden database backup to disk on startup
writing unit/integration tests
intensive manual testing

@kilrau we agreed on not implementing a restoring feature for now, right?

ghost

Skimmed through the code. Agree on the high level concept. Nice to see this is progressing well.

bin/xu-backup

kilrau · 2019-06-17T14:33:36Z

@kilrau we agreed on not implementing a restoring feature for now, right?

Correct, since it can be restored using lnd/raiden directly.

As far as I see it we'd have to implement lnds https://api.lightning.community/#restorechannelbackups in xud for that, track differences between btc & ltc ... but why not open an issue to keep track of it post-1.0.0 @michael1011 :)

sangaman

Looks good conceptually.

lib/backup/Backup.ts

ghost

Left some comments/questions above. As a general style nit I noticed a lot of blank lines which is not consistent with the rest of the codebase. I think our tslint settings even display a warning/suggestion against those?

lib/backup/Backup.ts

michael1011 · 2019-06-24T16:22:03Z

@erkarl regarding the style: you are right about the inconsistencies with the rest of the codebase but I didn't think that it mattered that much when actually writing the code since some other files also have some blank lines. And npm run lint didn't throw any warnings or messages so I just left it as it is now.

What do you think? Should I remove some blank lines or it is fine to leave it as is?

ghost · 2019-06-26T09:36:52Z

What do you think? Should I remove some blank lines or it is fine to leave it as is?

Just leave it as is for now. I'll take a look at our linter configuration at some point and unify the formatting style.

kilrau · 2019-07-03T14:23:02Z

Added xud.db, since we have quite important things in there (swap states). Loosing these means potentially loosing funds.

kilrau · 2019-08-23T13:31:05Z

Todo's need to be updated as per #917

kilrau

The filewatcher approach is nice!

Cosmetics:
Can we rename xu-backup -> xud-backup?

michael1011 · 2019-10-08T16:08:15Z

Can we rename xu-backup -> xud-backup?

Done!

sangaman

Just some minor nits/suggestions, otherwise looks very nice. I haven't tested it manually yet but I can run it tomorrow.

bin/xud-backup

lib/backup/Backup.ts

lib/lndclient/LndClient.ts

test/jest/Backup.spec.ts

lib/backup/Backup.ts

kilrau · 2019-10-16T08:58:44Z

let's definitely do some manual testing @sangaman @erkarl @michael1011

sangaman · 2019-10-17T15:55:47Z

bin/xud-backup

+      describe: 'Port for the lnd gRPC interface',
+      type: 'number',
+    },
+    'raiden.database': {


raiden.dbpath perhaps? Also would it make sense to add this property to Config.ts and RaidenClientConfig so it can be assigned a default value and get added automatically to the sample config?

I didn't do that initially because the path to the database of raiden depends not only on the network it is running on but also on a node specific id and looks like this: exchange/node_1a8e5ab4/netid_4321/network_de8a2bdd/v23_log.db.

Any ideas on how to determine a reasonable default value?

I think raiden.dbpath could be the only value where we don't specify a default value. Thoughts @sangaman?

I see, that makes sense. We can just leave this out then and not have a default value.

lib/backup/Backup.ts

sangaman

I'm not seeing the debug statements for when lnd channel backups are written - it should happen after swaps occur right? I tested with a BTC-LTC swap and I did see this.

24/10/2019 13:33:24.758 [BACKUP] verbose: Writing initial BTC LND channel backup
24/10/2019 13:33:24.769 [BACKUP] verbose: Listening to BTC LND channel backups
24/10/2019 13:33:24.778 [BACKUP] verbose: Writing initial LTC LND channel backup
24/10/2019 13:33:24.784 [BACKUP] verbose: Listening to LTC LND channel backups

One more suggestion is that it would be nice to be able to specify a path to the config file when it's not in the default location, which we could do using the --xudir option (the Config module looks for the config file in the xudir directory).

Lastly I'm also thinking it would be nice if on the first writes, it logs where it's saving the backup to, like Writing initial xud database backup to [path].

sangaman

I'm not seeing the debug statements for when lnd channel backups are written - it should happen after swaps occur right? I tested with a BTC-LTC swap and I did see this.

24/10/2019 13:33:24.758 [BACKUP] verbose: Writing initial BTC LND channel backup
24/10/2019 13:33:24.769 [BACKUP] verbose: Listening to BTC LND channel backups
24/10/2019 13:33:24.778 [BACKUP] verbose: Writing initial LTC LND channel backup
24/10/2019 13:33:24.784 [BACKUP] verbose: Listening to LTC LND channel backups

One more suggestion is that it would be nice to be able to specify a path to the config file when it's not in the default location, which we could do using the --xudir option (the Config module looks for the config file in the xudir directory).

Lastly I'm also thinking it would be nice if on the first writes, it logs where it's saving the backup to, like Writing initial xud database backup to [path].

michael1011 · 2019-10-24T18:40:37Z

I'm not seeing the debug statements for when lnd channel backups are written - it should happen after swaps occur right? I tested with a BTC-LTC swap and I did see this.

I am not backing up the database of LND but the SCBs (static channel backups). These backups are done once for a channel and are therefore static. This also means that a new backup will be done once you close an existing or open a new channel. When you do that it should print you a debug statement @sangaman

michael1011 · 2019-10-31T12:38:38Z

The tests fail only on Travis and because of this error:

 FAIL  test/jest/Backup.spec.ts

  ● Backup › should write Raiden backups on new event

    RangeError: Maximum call stack size exceeded

        at Function.[Symbol.hasInstance] (<anonymous>)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:200:14)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

      at WriteStream (../../node_modules/graceful-fs/graceful-fs.js:201:29)

It is an bug of one of the dependencies of Jest (jestjs/jest#9062) and was fixed recently.

What should be done about this? Wait for the next Jest release and continue working on the restore in this branch or should we update the dependency in npm-shrinkwrap.json ourselves?

@sangaman @erkarl

sangaman · 2019-10-31T16:41:10Z

Hmmm, it looks like jest doesn't release very often unfortunately. I think I'm fine with either approach, if you want to update the graceful-fs dependency directly, I'd say go for it. Or we can leave this alone until the next release, maybe they are due for one.

Another option would be to split the test case out of the commit, merge this without the test case, and then open a separate PR for the test case and merge that (along with a jest dependency update) once there's a new jest release. That actually might be my preferred approach.

michael1011 requested review from a user, sangaman and ImmanuelSegol June 14, 2019 14:25

ghost reviewed Jun 15, 2019

View reviewed changes

bin/xu-backup Outdated Show resolved Hide resolved

kilrau assigned michael1011 Jun 17, 2019

michael1011 mentioned this pull request Jun 17, 2019

Add restore feature to xu-backup #1041

Closed

sangaman reviewed Jun 18, 2019

View reviewed changes

lib/backup/Backup.ts Outdated Show resolved Hide resolved

lib/backup/Backup.ts Show resolved Hide resolved

michael1011 force-pushed the channel-backups branch 5 times, most recently from 1af6c81 to e50a079 Compare June 22, 2019 10:14

michael1011 marked this pull request as ready for review June 22, 2019 10:23

michael1011 requested review from a user and sangaman June 22, 2019 11:01

ghost reviewed Jun 24, 2019

View reviewed changes

lib/backup/Backup.ts Outdated Show resolved Hide resolved

lib/backup/Backup.ts Outdated Show resolved Hide resolved

lib/backup/Backup.ts Outdated Show resolved Hide resolved

michael1011 force-pushed the channel-backups branch from e50a079 to bfbf6d7 Compare June 24, 2019 16:13

michael1011 force-pushed the channel-backups branch 3 times, most recently from 58d1ade to 9f75a56 Compare October 7, 2019 20:27

michael1011 requested review from a user and kilrau October 7, 2019 20:27

kilrau added the P1 top priority label Oct 8, 2019

kilrau suggested changes Oct 8, 2019

View reviewed changes

michael1011 force-pushed the channel-backups branch from 9f75a56 to d9c4a02 Compare October 8, 2019 16:07

sangaman approved these changes Oct 15, 2019

View reviewed changes

michael1011 force-pushed the channel-backups branch from d9c4a02 to 1cd7476 Compare October 16, 2019 18:37

michael1011 requested review from sangaman and kilrau October 16, 2019 18:37

sangaman removed the request for review from ImmanuelSegol October 17, 2019 13:10

sangaman reviewed Oct 17, 2019

View reviewed changes

lib/backup/Backup.ts Outdated Show resolved Hide resolved

michael1011 force-pushed the channel-backups branch from 1cd7476 to e193697 Compare October 17, 2019 16:21

sangaman reviewed Oct 24, 2019

View reviewed changes

sangaman approved these changes Oct 24, 2019

View reviewed changes

michael1011 force-pushed the channel-backups branch 3 times, most recently from 0275ed4 to 2ca28ea Compare October 29, 2019 15:38

feat: add backup daemon

c32e804

michael1011 force-pushed the channel-backups branch from 2ca28ea to c32e804 Compare November 2, 2019 10:14

kilrau approved these changes Nov 2, 2019

View reviewed changes

michael1011 merged commit 225d61a into master Nov 2, 2019

michael1011 deleted the channel-backups branch November 2, 2019 10:44

michael1011 restored the channel-backups branch November 2, 2019 10:45

michael1011 deleted the channel-backups branch November 2, 2019 10:45

This was referenced Dec 12, 2019

Integrate Backup & Restore Feature ExchangeUnion/xud-docker#245

Closed

test: add test case for backup feature #1353

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add backup daemon #1032

feat: add backup daemon #1032

michael1011 commented Jun 14, 2019 •

edited

Loading

ghost left a comment

kilrau commented Jun 17, 2019

sangaman left a comment

ghost left a comment

michael1011 commented Jun 24, 2019

ghost commented Jun 26, 2019

kilrau commented Jul 3, 2019 •

edited

Loading

kilrau commented Aug 23, 2019

kilrau left a comment

michael1011 commented Oct 8, 2019

sangaman left a comment

kilrau commented Oct 16, 2019 •

edited

Loading

sangaman Oct 17, 2019

michael1011 Oct 17, 2019

ghost Oct 23, 2019

sangaman Oct 24, 2019

sangaman left a comment

sangaman left a comment

michael1011 commented Oct 24, 2019

michael1011 commented Oct 31, 2019

sangaman commented Oct 31, 2019

feat: add backup daemon #1032

feat: add backup daemon #1032

Conversation

michael1011 commented Jun 14, 2019 • edited Loading

ghost left a comment

Choose a reason for hiding this comment

kilrau commented Jun 17, 2019

sangaman left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

michael1011 commented Jun 24, 2019

ghost commented Jun 26, 2019

kilrau commented Jul 3, 2019 • edited Loading

kilrau commented Aug 23, 2019

kilrau left a comment

Choose a reason for hiding this comment

michael1011 commented Oct 8, 2019

sangaman left a comment

Choose a reason for hiding this comment

kilrau commented Oct 16, 2019 • edited Loading

sangaman Oct 17, 2019

Choose a reason for hiding this comment

michael1011 Oct 17, 2019

Choose a reason for hiding this comment

ghost Oct 23, 2019

Choose a reason for hiding this comment

sangaman Oct 24, 2019

Choose a reason for hiding this comment

sangaman left a comment

Choose a reason for hiding this comment

sangaman left a comment

Choose a reason for hiding this comment

michael1011 commented Oct 24, 2019

michael1011 commented Oct 31, 2019

sangaman commented Oct 31, 2019

michael1011 commented Jun 14, 2019 •

edited

Loading

kilrau commented Jul 3, 2019 •

edited

Loading

kilrau commented Oct 16, 2019 •

edited

Loading