smogon / pokemon-showdown

Pokémon battle simulator.
https://pokemonshowdown.com
MIT License
4.77k stars 2.78k forks source link

Implementing SQLite support #3773

Open Morfent opened 7 years ago

Morfent commented 7 years ago

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 in config/chatrooms.json.

There are two packages to consider using for SQLite support: node-sqlite3 and better-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 using better-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 for node-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:

Zarel commented 7 years ago

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...

Morfent commented 7 years ago

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

Zarel commented 7 years ago

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.

KamilaBorowska commented 7 years ago

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".

Zarel commented 7 years ago

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.

AnnikaCodes commented 3 years ago

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 RoomDataStorage interface so we can implement a SQLite backend later, potentially alongside a JSON backend and/or a mocked backend for unit tests.