TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
980 stars 309 forks source link

Use Redis 7.0 features #5269

Open adriansmares opened 2 years ago

adriansmares commented 2 years ago

Summary

We should use Redis 7.0 features once 7.0 compatibility is widely available.

Why do we need this?

Certain operations can be more efficient this way.

What is already there? What do you see now?

  1. FindProtos in pkg/redis uses SORT. SORT cannot be used against a read only replica, due to the potential STORE parameter (which we don't use).
  2. XAUTOCLAIM will automatically skip deleted messages (see https://github.com/redis/redis/pull/10227).

What is missing? What do you want to see?

  1. FindProtos should use SORT_RO.
  2. We can remove the nil check in the Redis task dispatch script.

Environment

3.18.1

How do you propose to implement this?

  1. Update SORT command usage.
  2. Just drop the nil check.

How do you propose to test this?

UT.

Can you do this yourself and submit a Pull Request?

Yes.

johanstokking commented 2 years ago

We're a bit tied to the managed Redis offering from the major cloud vendors. They're generally a bit slow with upgrading Redis. Specifically Microsoft Azure is far behind.

Worst case, we have a backwards compatibility mode built in the Redis stores. For instance, we could take the read only client and use SORT_RO when the reported Redis version is 7.0 or higher, and otherwise the writer client and SORT?

adriansmares commented 2 years ago

I've created this issue mostly for housekeeping. I know we won't have 7.0 for a long time.

Until we have 7.0 compatibility, I've written a read only compatible SORT-ish here - https://github.com/TheThingsIndustries/lorawan-stack/blob/26406f658dbbc29b03c3c8d2fe05ac650209e4aa/pkg/redis/redis.tti.go#L31 .