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

[feature request] sqlite3 backend support #170

Open
JssDWt opened this issue Feb 27, 2023 · 2 comments
Open

[feature request] sqlite3 backend support #170

JssDWt opened this issue Feb 27, 2023 · 2 comments

Comments

@JssDWt
Copy link
Contributor

JssDWt commented Feb 27, 2023

Add a (default) option to use a sqlite3 database in favor of bbolt.

bbolt doesn't offer functionality like indexes. Also database compaction is an issue where you'll have to periodically shut down peerswap in order to compact the database for performance.

I suggest to

  1. wrap the different stores in interfaces.
  2. Add a sqlite3 implementation to those interfaces.
  3. Add a flag to specify which database to use. Where the default will be sqlite3.

Open questions:

  • Do we need to add a migration from bbolt to sqlite3 for existing users using bbolt right now?
  • Changing the default database would affect existing users that already use a bbolt backend. Will docs about this change be enough?
  • Do we remove bbolt support entirely?

NOTE: I will be working on implementing this myself.

@nepet
Copy link
Contributor

nepet commented Feb 27, 2023

Hey Jesse, welcome to PeerSwap!

Your suggestion seems like a solid approach to add sqlite3 to the project. I am on your page to use sqlite3 as the default database instead of bbolt.

I don't think that we need a migration from bbolt to sqlite3 as the data is (besides some historical knowledge/statistics the operator might be interested in) useless as soon as a swap terminates. This being said, we could argue to drop bbolt support altogether. This could also give us the chance to have a look at our data models and refine them as needed. There is a lot of clutter that could be simplified using relations, making it easier to maintain the project and providing support in the future.

We currently have the unwritten rule to prevent an update if we detect an ongoing swap. Maybe we should check on the implementation of this guard before we release the sqlite3 support, just in case.

Any thoughts on this @wtogami?

@JssDWt
Copy link
Contributor Author

JssDWt commented Mar 13, 2023

This could also give us the chance to have a look at our data models and refine them as needed. There is a lot of clutter that could be simplified using relations, making it easier to maintain the project and providing support in the future.

About this: I'm thinking to first replicate the bbolt behavior in the sqlite implementation. So there will just be the Get and Update method. It will be a bit cumbersome in a relational world at first, but then later we can redefine how the data layer is used and only update the fields that need updating. I'm not sure yet what is used where exactly (like these messages that are persisted in the SwapData struct), so I figured I'd just persist and retrieve all of it for the first iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants