Skip to content

docs: add notes on contents of Postgres DB as Key-Value store to Postgres.md#7442

Merged
Roasbeef merged 1 commit intolightningnetwork:masterfrom
84adam:postgres-docs-schema-84adam
Apr 10, 2023
Merged

docs: add notes on contents of Postgres DB as Key-Value store to Postgres.md#7442
Roasbeef merged 1 commit intolightningnetwork:masterfrom
84adam:postgres-docs-schema-84adam

Conversation

@84adam
Copy link
Copy Markdown
Contributor

@84adam 84adam commented Feb 23, 2023

Change Description

Adding notes on how Postgres DB is currently used as a Key-Value store, and which tables are included.

Addresses Issue #6619 - "Missing documentation for postgresql database structure"

Steps to Test

Key/Value pairs are not all TLV-encoded. Schema/structure is unclear (outside of K/V column names). However, some Key Names and Values are possible to decode into human-readable strings. For example: I used Python + psycopg2 + text processing to decode some of these, such as Node names (see comment on Gist): https://gist.github.com/84adam/d85f5f40e8821925585f263f68b655eb

SQL statements do not reveal any consistent encoding or structure:

select * from channeldb_kv limit 5;
                  key                   | value | parent_id | id | sequence 
----------------------------------------+-------+-----------+----+----------
 \x6f70656e2d6368616e2d6275636b6574     |       |           |  1 |         
 \x636c6f7365642d6368616e2d6275636b6574 |       |           |  2 |         
 \x636972637569742d6677642d6c6f67       |       |           |  3 |         
 \x6677642d7061636b61676573             |       |           |  4 |         
 \x696e766f69636573                     |       |           |  5 |         
select * from walletdb_kv limit 5;
        key         | value | parent_id | id | sequence 
--------------------+-------+-----------+----+----------
 \x77616464726d6772 |       |           |  1 |         
 \x7774786d6772     |       |           |  2 |         
 \x6d61696e         |       |         1 |  3 |         
 \x73796e63         |       |         1 |  4 |         
 \x73636f7065       |       |         1 |  5 | 

This documentation update attempts to make clear that users cannot expect consistency for now in issuing SQL queries to their Postgres databases, and points out where more documentation will be required once additional schema is introduced into these tables.

Copy link
Copy Markdown
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @84adam!

Pls see our contributor guidelines for how to structure your commit title.

So would suggest something like: docs: add <short description here>

Also please add an entry to the release notes

@84adam 84adam changed the title Update Postgres.md: add notes on contents of Postgres DB as Key-Value store docs: add notes on contents of Postgres DB as Key-Value store to Postgres.md Feb 23, 2023
@84adam
Copy link
Copy Markdown
Contributor Author

84adam commented Feb 23, 2023

Thank you, @ellemouton. Updated.

Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! @ellemouton 's comment is not addressed thought,

So would suggest something like: docs: add

So the git commit messages need to be updated as suggested.

@84adam 84adam force-pushed the postgres-docs-schema-84adam branch from bc20229 to 69ce385 Compare February 23, 2023 20:52
@84adam
Copy link
Copy Markdown
Contributor Author

84adam commented Feb 23, 2023

Thanks, @yyforyongyu. I believe I have corrected this now.

@84adam 84adam force-pushed the postgres-docs-schema-84adam branch from 69ce385 to 21bc1a1 Compare February 24, 2023 15:14
@84adam
Copy link
Copy Markdown
Contributor Author

84adam commented Feb 27, 2023

@yyforyongyu I believe the requested changes have been completed. (Squashed commits.) Please let me know if I'm missing anything else. Thanks!

@84adam 84adam force-pushed the postgres-docs-schema-84adam branch from 21bc1a1 to 713dece Compare March 1, 2023 04:37
@84adam
Copy link
Copy Markdown
Contributor Author

84adam commented Mar 1, 2023

Removed previous "references" and squashed commits.

- https://twitter.com/positiveblue2/status/1621170766808256512 - "Currently LND supports Postgres but it is used as K/V store. Introducing better schemas will unlock better performance and make easier to get insights from our nodes."
- https://twitter.com/lndreviewclub/status/1618696557402152961 - "Postgres support was added in lnd 0.14, but the next step is to convert the invoices to a native SQL schema."
- https://lightningcommunity.slack.com/archives/C03ECAP7HK6/p1675357998163889?thread_ts=1675357814.564459&cid=C03ECAP7HK6 - 'Values are a mix of TLV and non-TLV-encoded data. Key/Values are just stored as plain byte slices. With more schema introduced it'll gain a lot of performance over time. This methodology allows for incrementally upgrading the DB code over time. Invoice DB transactions do not touch data in other buckets (they are self contained).'
- https://gist.github.com/84adam/d85f5f40e8821925585f263f68b655eb - Python script to decode keys/values from Postgres LND database

Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

LGTM✅ Thanks again for the PR!

Copy link
Copy Markdown
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

Looks good! I think it could be worth adding an entry to sqlite.md too. Or at least linking to this explanation from there?

@84adam
Copy link
Copy Markdown
Contributor Author

84adam commented Apr 7, 2023

Anything I'm missing for this PR? Curious if you have any idea when it might be merged? @ellemouton @yyforyongyu

Thanks!

@yyforyongyu yyforyongyu added this to the v0.16.1 milestone Apr 10, 2023
@yyforyongyu
Copy link
Copy Markdown
Member

@84adam added to 0.16.1 so this will be merged before that release.

@saubyk saubyk added documentation Documentation changes that do not affect code behaviour postgres labels Apr 10, 2023
@Roasbeef Roasbeef merged commit 9580584 into lightningnetwork:master Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation changes that do not affect code behaviour no-itest postgres

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants