valkey-io / valkey

A flexible distributed key-value datastore that supports both caching and beyond caching workloads.
https://valkey.io
Other
15.5k stars 570 forks source link

Get rid of trademark #25

Open zuiderkwast opened 5 months ago

zuiderkwast commented 5 months ago

Get rid of the Redis name, except where it's needed for API compatibility. This can be done once we have a name.

Some of the below changes have a low risk of breaking e.g. script parsing logs or INFO output, but I'd say those are acceptable.

hwware commented 5 months ago

REDISMODULE will be changed as well? such as XXXMODULE? ^_^

zuiderkwast commented 5 months ago

REDISMODULE will be changed as well? such as XXXMODULE? ^_^

We break existing modules if we do that, right? We can provide a compatibility layer of some sort, deprecate the old names and phase then out after 1-2 major versions...?

hpatro commented 5 months ago

Changes to handle trademark violation

External Facing

  1. Clients
    1. Do we need separate client(s)?
  2. Compatibility (what are we compatible with?)
    1. Can we advertise “RESP” (Redis Serialization Protocol) compatibility?
    2. API compatibility (client agnostic?)
    3. Data (Snapshot/Replication)
      1. RDB compatibility
        1. import/export rdb
        2. versioning and identification (if we change to different numbering)
      2. PSYNC
      3. AOF
    4. Cluster Compatibility i. Redis Cluster begins with “RCmb” header stands for Redis Cluster Message Bus.
    5. Modules API
    6. LUA
  3. Monitoring and Metrics
    1. Redis INFO metrics
      1. Server information
  4. Commands
    1. HELLO response
      1. server name - Redis
      2. server version - x.x.x

Internal code changes

  1. Binaries/tooling
    1. redis-server
    2. redis-cli
      1. Tooling name
      2. connection scheme
    3. redis-benchmark
    4. redis-trib
    5. redis-check-rdb
    6. redis-check-aof
    7. redis-sentinel
  2. Dependencies
    1. hiredis
  3. Shared Objects
    1. redis-tls.so
  4. Configuration
    1. redis.conf
      1. self name
      2. config - pidfile path
      3. config - aclfile path
    2. version.h
  5. Child Process(es)
    1. redis-aof-rewrite
  6. Code comments (2986 occurence in 381 files)
  7. Function and variable naming (2986 occurence in 381 files)
  8. Module API changes in test modules

I was gathering information on similar lines. Feel free to update the top level if you find anything interesting.

zuiderkwast commented 5 months ago

There are lots of questions here. I think we need to ask some law folks or check the source code of other forks, how much they changed.

zuiderkwast commented 5 months ago

https://dba.stackexchange.com/questions/149586/why-does-mariadb-still-use-the-mysql-name-everywhere-within-the-file-system

zuiderkwast commented 5 months ago

MariaDB obviously didn't rename everything at once.

mysql is a simple SQL shell (with GNU readline capabilities). MariaDB starting with 10.4.6

From MariaDB 10.4.6, mariadb is a symlink to mysql. MariaDB starting with 10.5.2

From MariaDB 10.5.2, mariadb is the name of the script, with mysql a symlink .

PingXie commented 5 months ago

REDISMODULE will be changed as well? such as XXXMODULE? ^_^

We break existing modules if we do that, right? We can provide a compatibility layer of some sort, deprecate the old names and phase then out after 1-2 major versions...?

Changing REDISMODULE_* should be fine. The only binding contract is the API prefix and the layout of the relevant structures such as RedisModule_GetApi being the first entry of the module ctx. We can freely change the struct/field names.

On module load, the existing modules will be looking for RedisModule_* APIs. We can double-register on the server side with both prefixes (RedisModule_ and XXXXModule_) but only expose the binding to XXXXModule_ in the new module header. This way, modules compiled against our codebase will use the new API prefix and modules compiled against the Redis codebase will still be able to locate these functions using the old prefix.

The higher level question though is the semantics of these APIs. I don't foresee this being a problem in the near term. Overtime, however, behavior will diverge for sure, regardless of our efforts. There will be a point where such compatibility is no longer needed nor desired but this will take quite some time, i.e. years if I may guess.

PingXie commented 5 months ago

There are lots of questions here. I think we need to ask some law folks or check the source code of other forks, how much they changed.

Agreed and I like your approach of making incremental and small changes one at a time.

hpatro commented 5 months ago

On module load, the existing modules will be looking for RedisModule_* APIs. We can double-register on the server side with both prefixes (RedisModule_ and XXXXModule_) but only expose the binding to XXXXModule_ in the new module header. This way, modules compiled against our codebase will use the new API prefix and modules compiled against the Redis codebase will still be able to locate these functions using the old prefix.

So, registering on the server side for RedisModule_ wouldn't be a trademark violation?

PingXie commented 5 months ago

So, registering on the server side for RedisModule_ wouldn't be a trademark violation?

This goes back to @zuiderkwast's earlier comment (https://github.com/placeholderkv/placeholderkv/issues/25#issuecomment-2018976668).

I guess one option could be to make it optional, i.e., controlled by a "compat" config, which could be used more broadly too. This way, we don't register RedisModule_ by default and the decision would be left to the end user.

hpatro commented 5 months ago

So, registering on the server side for RedisModule_ wouldn't be a trademark violation?

This goes back to @zuiderkwast's earlier comment (#25 (comment)).

I guess one option could be to make it optional, i.e., controlled by a "compat" config, which could be used more broadly too. This way, we don't register RedisModule_ by default and the decision would be left to the end user.

Yeah, will be good to understand this. We're walking a tightrope here. compatibility vs getting sued.

zuiderkwast commented 5 months ago

So, registering on the server side for RedisModule_ wouldn't be a trademark violation?

I'm not sure, but it might be "fair use" or something to keep backward compatibility. I'm not sure.

Renaming REDISMODULE macros and structs still makes us ABI-compatible, but not source code compatible, i.e. old modules can't just include our placeholdermodule.h file and expect their module to compile. But that may not be a huge issue either. We can give them a one-liner sed script to update their module code.

madolson commented 5 months ago

Do we want to create a compatibility layer for the lua. You can do use either placeholderkv.call() and redis.call()? It seems like we're also proposing the same thing for modules. Being able to have either RedisModule_ and PlaceHolderKV_ both generate ABI compatible files.

Cluster Compatibility Redis Cluster begins with “RCmb” header stands for Redis Cluster Message Bus.

Since this is part of the contract, I think we can leave it.

saolof commented 5 months ago

I'm a bit confused. The timeline for trademarks seems to be roughly this:

2009: Redis is publicly released as an open source project. 2011: Redis labs is founded. 2018: Redis labs ltd filed a trademark application for some uses of Redis (was opposed by Rackspace at the time). 2021: Redis labs changes name to Redis. Last month: Redis ltd files a trademark application for Redis-the-database (still pending) .

If they thought that they actually had exclusive rights on Redis the database, why would they have filed a last-minute trademark application?

hwware commented 5 months ago

Another 2 issues we need to change:

  1. RESP (Redis Serialization Protocol) We need change.
  2. RDB file rename: RDB represents Redis Database
parthpatel commented 5 months ago

What is the conclusion for Redis modules and LUA scripting? If I understand it correctly, the changes proposed mainly allow new modules and lua script invocations to use new prefix instead of Redis* prefix. I would say, that's not a high priority currently given large Redis user-base. We can take it up after launching first stable version that's fully compatible with 7.2 version (or the version that we decide).

natoscott commented 5 months ago

So, registering on the server side for RedisModule_ wouldn't be a trademark violation?

I'm not sure, but it might be "fair use" or something to keep backward compatibility. I'm not sure.

My understanding is that where we are providing an implementation of an interface (and this goes for APIs, protocols, on-disk formats, etc) it is very difficult for some other party to argue that this is protected (by either copyright or trademark).

madolson commented 5 months ago

My understanding is that where we are providing an implementation of an interface (and this goes for APIs, protocols, on-disk formats, etc) it is very difficult for some other party to argue that this is protected (by either copyright or trademark).

This is the same guidance I've been given. If we're implementing against or providing a functional interface, it's unlikely to hold up. I still think we should aim to remove the Redis mark from our project, but we should do so methodically and carefully, leaving behind what we believe is necessary.

natoscott commented 5 months ago

Another 2 issues we need to change:

1. RESP (Redis Serialization Protocol)  We need change.

2. RDB file rename:  RDB represents Redis Database

If the references are directly to "RESP" (the network protocol) and "RDB" (the on-disk format) they are not a problem and IMO these should not be changed. They are important points of compatibility with the previous project.

madolson commented 5 months ago

@hwware Can you crate two separate issues for how we might break RDB and RESP in the future though. We have a problem with the RDB version, in that we can't stay compatible with the Redis community edition. At some point we'll need to change the 🪄 https://github.com/placeholderkv/placeholderkv/blob/unstable/src/rdb.c#L1395.

For RESP, it seems less likely this will change. But, we should at least have an issue pinned for discussion about it.

yangbodong22011 commented 5 months ago

Moved from here, As far as I know, redis_mode is used in many places:

  1. StackExchange.Redis: Determine the startup mode through redis_mode as standalone, cluster or sentinel. https://github.com/StackExchange/StackExchange.Redis/blob/main/src/StackExchange.Redis/ResultProcessor.cs#L872

  2. Prometheus Exporter, use redis_mode to display instance_info. https://github.com/oliver006/redis_exporter/blob/master/exporter/info.go#L168

I think we should start a separate issues to let sdk/tool developers report their commands or fields that depend on Redis. (If possible, the best approach is to not change these old fields)

zuiderkwast commented 5 months ago

@yangbodong22011 We have no plans on removing "redis_mode". Where did you get from?

yangbodong22011 commented 5 months ago

@yangbodong22011 We have no plans on removing "redis_mode". Where did you get from?

Just some of the risk points that I can think of that may not cause compatibility.