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

Discussion: Ecto for Cache Adapter? #329

Closed
khionu opened this issue Sep 24, 2021 · 4 comments
Closed

Discussion: Ecto for Cache Adapter? #329

khionu opened this issue Sep 24, 2021 · 4 comments

Comments

@khionu
Copy link
Contributor

khionu commented Sep 24, 2021

I'll update this issue as the discussion unfolds

Motive

The core Ecto library would make it much easier for cache backends to be implemented without having to recreate features like query filter.

Pros

  • Existing backends for MySQL, SQLite, Postgres, ETS, and Mnesia
  • Allows us to own more of the logic
    • All of the logic for existing backends

Cons

  • Imposes dependency on Ecto
  • Imposes requirement to implement Ecto.Adapter and associated behaviours if backend doesn't exist
  • No Redis adapter for Ecto (is this really a con?)

Considerations

  • All types that can be sent back to Discord must have the Ecto fields excluded during serialization. There may need to be some adjustments for deserialization as well

Conclusion

TBD

@khionu
Copy link
Contributor Author

khionu commented Sep 24, 2021

Going by some napkin math and R Danny's metrics, a DB would be a bottle neck before Ecto. We should be good for thousands of events per second, and R Danny gets that per minute, and that's without being selective of intents.

@jchristgit
Copy link
Collaborator

I would be very happy to hear other opinions, but personally I'm not really sold on using Ecto for our cache here. It is a great library, but I don't think it's suited for usage in a cache type of way. You mentioned that:

  • Existing backends for MySQL, SQLite, Postgres, ETS, and Mnesia. MySQL, SQLite and PostgreSQL are databases which persist data to disk, so our cache adapters would have to implement logic to purge / synchronize the cache on startup. These three aside, the mnesia adapter does not support Ecto 3, which we will probably want to use. The ETS adapter (assuming you mean etso) does look nice, but then (for me personally) we would only have one backend, and implementing other backends would require knowledge of how Ecto works, not to mention that it also incurs further investments to keep the implementation updated with Ecto (although from knowing Elixir, this should not be too hard)
  • Allows us to own more of the logic. I don't understand this point. Don't we own all the logic for cache adapters right now?
  • No Redis adapter for Ecto (is this really a con?). I would claim this is a con, as Redis better fits the caching usecase then MySQL, SQLite and PostgreSQL, IMO. With our current implementation, it's straightforward to implement a Redis cache adapter.

@khionu
Copy link
Contributor Author

khionu commented Sep 25, 2021

mnesia adapter does not support Ecto 3

There is one that does

so our cache adapters would have to implement logic to purge / synchronize the cache on startup

The point of people using those would be persistence. I don't think purging should be in scope. If individual implementations of the adapter want to do that, that should be totally up to them.

I would claim this is a con, as Redis better fits the caching usecase then MySQL, SQLite and PostgreSQL, IMO.

I'd bet on most people using Nostrum to not be using Redis. For how few people who would use Redis over Mnesia or ETS, I'm not sure I would personally consider it a hard blocker, especially since using Ecto wouldn't strictly exclude Redis, just require creating an Ecto Adapter for it, which could wrap around one of the couple of Redis libraries out there. Sure, that is not an insignificant requirement, but it is one of the only cases where it is required.

I don't understand this point. Don't we own all the logic for cache adapters right now?

For ones we implement, yes. If someone is making their own, far from it. With Ecto, everything Discord/Nostrum specific would be already taken care of through our construction of Ecto.ChangeSets and our schemas.

Lastly, this is a Discord bot library. It would take an extremely large bot to not be able to use Postgres as a cache due to performance. I would argue viability as a cache of any DB is a non-issue here.


As stated on Discord, it is possible for this to be implemented as a third-party library. I will probably do that if we still consider someone having to wrap a Redis library as an Ecto adapter a hard blocker.

@jchristgit
Copy link
Collaborator

With the current cache adapters being included in the 0.5 release and settling on a custom approach instead with behaviours, we won't be using Ecto for cache adapters, but custom implementations are free to extend the cache adapter behaviour however they want.

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

No branches or pull requests

2 participants