-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implementing SQLite support #3773
Comments
We're going to have so many storage backends, since I also want to have Redis for short-term data and Cassandra for some long-term storage... |
Is SQLite still something that you'd want to have available in PS in that case? If not, I could put my time towards getting Cassandra implemented instead, unless that's the kind of feature that you'd need to implement yourself |
So, I was envisioning Cassandra would be used for battle logs and room logs and the replay server. Cassandra is mostly only good for key-value storage. A user database is also kind of useful on it. But I think for most persistence we actually do want SQLite. |
Cannot we just have one or two storage backends? Having more configuration makes maintanence and testing harder. Preferably one, but two is fine if the distinction is between "needs additional configuration like gcc compiler installed" vs "doesn't need additional configuration but is somehow worse". |
The problem is, they have different goals. Redis's use is to bypass Node's 1GB RAM limit. Cassandra's use is as a key-value store for huge amounts of data. SQLite is for every server to store miscellaneous data. |
We now have a SQLite library thanks to Mia, and modlogs and Trivia both use it. I'm fine with migrating chatroom data to a database as well, but I don't know how necessary it is - do we really store so much data about rooms that it's unwieldy to use JSON? In the past I've heard concerns voiced about not being able to run the server at all without SQLite; making rooms require SQLite would make this more difficult. Maybe we should refactor to use a generic |
Now that we can use
async/await
syntax, I feel the issues that I was having writing a SQLite database for the trivia plugin without it turning into spaghetti code are alleviated. Writing a database just for the chat plugin doesn't seem like a good idea to me though, since there are many other parts of the sim that could really use proper databases over using JSON (whose writers still leak on write, though not quite as badly as before), CSV, TSV, and custom formats used for some files. Global chatroom data would especially benefit from using a SQLite database because of the large amount of data kept inconfig/chatrooms.json
.There are two packages to consider using for SQLite support:
node-sqlite3
andbetter-sqlite3
.node-sqlite3
uses a mainly asynchronous API, though queries can be serialized and parallelized where needed.better-sqlite3
uses a mainly synchronous API, and claims to have better performance than the former. I'm leaning towards usingbetter-sqlite3
, but I'm rather wary of how it boasts letting Node's GC handle memory management rather than the C++ portion of the module, since it could potentially open the possibility for memory leaks, but I'll look into that on my own time to see whether or not that's a real issue.Installing either module on Windows is a bit of a problem, since they both use
node-gyp
and have their own dependencies that need to be installed on the system as admin (like Python fornode-sqlite3
) and requires some fiddling with environment variables in order to get the package to compile properly. If SQLite becomes a mandatory dependency, then I'll likely have to write a guide on how to get the package installed.Anyway, I think we should aim to make it possible for devs with little knowledge of SQL to be able to make use of SQLite databases. Creating a
database.js
module that creates an ORM abstraction over the database (while still allowing raw SQL queries to be used for more complex tasks) might be the best approach to this. Ideally using a dependency to handle this for us would be better, but none seems to exist at the moment. I might end up writing one myself and making it a dependency for PS instead.My main concerns are this:
The text was updated successfully, but these errors were encountered: