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

Mark Redis database backend as deprecated #14679

Merged
merged 1 commit into from
May 26, 2024
Merged

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented May 17, 2024

Context
Redis isn't really a good key-value database but more of a cache.
This is evidenced by how it presents itself and stuff like eviction algorithms, which would cause terrible data loss if applied to Minetest worlds, as well as keeping all data in RAM, which is inconvenient and unnecessary for MT.
A big selling point for redis is also distributedness, which is relevant for web applications (lots of concurrent processes, clusters, ...) whereas Minetest is a single-threaded, single-instance application.

Description
There's probably nobody really using redis so I propose deprecating and eventually removing it.
This PR both adds a deprecation message and will automatically turn all Redis access read-only with 5.10.0 (or whichever version is next).

Alternatives
People who require a networked database can switch to Postgres.
People who don't can switch to SQLite or LevelDB.
Worth noting that both Postgres (own implementation) and SQLite (via the OS) keep often used data right in RAM, so this isn't a property exclusive to Redis.
If someone absolutely wanted to remove disk access latency from world loading they can use sqlite on tmpfs and I'm sure postgres can also can also be configured in this manner.

@sfan5 sfan5 added @ Server / Client / Env. Discussion Issues meant for discussion of one or more proposals labels May 17, 2024
@appgurueu
Copy link
Contributor

There's also the "dummy" backend for servers like CTF which save the static map data that needs to be persisted in schematics and want the relatively small map to be very fast at runtime, hence want to keep it entirely in RAM.

Considering this, it seems very likely to me that we can deprecate Redis. If it turns out that we missed a use case, the affected people will complain about the deprecation, so that we can reconsider it. (Ideally, they would complain about it right now, but it is likely that this discussion doesn't reach them, and likely isn't worth the effort to seek them out.)

@sfan5 sfan5 added this to the 5.9.0 milestone May 20, 2024
@sfan5
Copy link
Member Author

sfan5 commented May 20, 2024

If it turns out that we missed a use case, the affected people will complain about the deprecation, so that we can reconsider it.

Someone could go and post this on the forums and/or Discord. It's more likely to be seen by relevant people that way.

@wsor4035
Copy link
Contributor

Someone could go and post this on the forums and/or Discord. It's more likely to be seen by relevant people that way.

already was posted in the minetest discord shortly after this pr was made. just posted it to a more targeted group as well

@sfan5 sfan5 merged commit 83bc362 into minetest:master May 26, 2024
13 checks passed
@sfan5 sfan5 deleted the noredis branch May 26, 2024 12:28
@sfan5 sfan5 removed the Discussion Issues meant for discussion of one or more proposals label May 26, 2024
@sfan5
Copy link
Member Author

sfan5 commented May 26, 2024

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

Successfully merging this pull request may close these issues.

None yet

3 participants