quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.57k stars 2.63k forks source link

Support for FalkorDB instead of (now deprecated) RedisGraph? #40092

Closed flowt-au closed 4 months ago

flowt-au commented 5 months ago

Description

Hi. I am using the io.quarkus.redis.datasource package and have migrated from using RedisGraph to FalkorDB. RedisGraph is deprecated and FalkorDB is a fork of that but has extra functionality compared to RedisGraph.

All works really well using the io.quarkus.redis.datasource with FalkorDB running in a container (falkordb/falkordb:v4.0.7) on the appropriate port / network address.

However, the new FalkorDB functionality is not available via the Quarkus Reactive Key Commands. For example RedisGraph did not support the copy key command whereas FalkorDB 4.0.7 does. Falkor also has vector db functionality that RedisGraph does not.

So, for example using the rename and copy key commands (simplified code):

import io.quarkus.redis.datasource.ReactiveRedisDataSource;

public void init(ReactiveRedisDataSource reactiveRedisDS) {
    // Reactive key commands. 
    this.keyClient = reactiveRedisDS.key();
}

// This works to rename a key
return keyClient.rename(oldKey, newKey).onItem().transform(i -> i == null ? true : false);

// But this copy command (which does exist on the keyClient but is not supported by RedisGraph) 
// throws "ERR not supported for this module key"
return keyClient.copy(oldKey, newKey).onItem().transform(i -> i == null ? true : false);

I am guessing I could use the Vertx Redis Client directly (since FalkorDB docs says it should work with existing Redis Clients) or maybe it needs some kind of custom client? Or even worse maybe I need to build my own Quarkus extension which would be a steep learning curve for me! ;-)

Anyway, given RedisGraph has been deprecated I was wondering if there is any thought to having a reactive FalkorDB client?

Thanks, Murray

Implementation ideas

No response

quarkus-bot[bot] commented 5 months ago

/cc @Ladicek (redis), @cescoffier (redis), @machi1990 (redis)

cescoffier commented 5 months ago

I don't understand how the copy command is related to Redis Graph. It seems, according to your example, to be a command of the group key.

If copy is a command supported by the base Redis, we can add it.

flowt-au commented 5 months ago

Thanks Clement. I will go back and look again. I think I misunderstood something because I am not clear now. Sorry. ;-)

cescoffier commented 5 months ago

I confirm, COPY is the base command: https://redis.io/docs/latest/commands/copy/

gkorland commented 5 months ago

COPY is not supported by FalkorDB only GRAPH.COPY see: https://docs.falkordb.com/commands/graph.copy.html

flowt-au commented 5 months ago

Thanks @cescoffier and @gkorland. I will do some more testing to see how to best get the new FalkorDB functionality without completely abandoning the Quarkus approach. I will try using the Vertx client directly for the "new" Falkor functionality.

Much appreciated, Murray

cescoffier commented 4 months ago

There are two approaches:

Thus, I would go with the first approach. We can create a quarkiverse repository to host the extension if you are willing to contribute.

flowt-au commented 4 months ago

@cescoffier Thanks Clement. I have started on the first suggestion.

As a starting point I am building a class that wraps JFalkorDB to provide both blocking and reactive methods. So far, so good. JFalkorDB wraps Jedis. I was hoping to make a "drop-in" replacement for the Quarkus Redis Datasource but since JFalkor returns different classes in the query response than does the Quarkus / Vertx response I have elected to go with the JFalkorDB class pattern. Processing the responses shouldn't be too different, mostly just different method names so VSCode / CoPilot should be of help there.

I will test against my current code base and graphs to confirm and document the migration path. Hopefully it won't be too messy! I have named things so as not to clash with the Quarkus Redis classnames and my plan is to implement both so I can incrementally convert my code, then remove the Redis extension. That means others could still use the Quarkus Redis extension for the other Redis modules and the Quarkus Falkor extension for graph.

Once that works I will read the "Write my own Quarkus Extension" guide and see how I go with that. Then yes, I would love to hand it over to the experts so others can benefit! ;-)

FalkorDB is looking very promising. One soon-to-be-released change supports maps / "nested json" as node properties so that will allow a graph-based JSON store type of thing which I am hanging out for.

Thanks again for your help and advice. Murray

cescoffier commented 4 months ago

Unfortunately Jedis is not really reactive, in the sense that it dispatches on its own thread pool and does not adhere to the event loop model used by Quarkus.

It is still possible to use it, but will require some wrapping to always go back to the event loop.

flowt-au commented 4 months ago

Yes, so this is what I am doing as a quick and dirty test so far, which works, but I haven't been watching the thread, I must admit.

The blocking query:

public ResultSet graphQuery(String key, String query) {
  ResultSet result = null;
  try {
    Graph graph = driver.graph(key);

    ResultSet response = graph.query(query);

    // Debugging code removed...

    // Return the response
    result = response;
  } catch (Exception e) {
    throw e;
  }
  return result;
}

The reactive version:

    private FalkorGraphCommands blocking;

    public ReactiveFalkorGraphCommands(FalkorGraphCommands falkorGraphCommandsBlocking) {
      this.blocking = falkorGraphCommandsBlocking;
    }

    public Uni<ResultSet> graphQuery(String key, String query) {
      return Uni.createFrom().item(() -> {
        return blocking.graphQuery(key, query);
      });
    }

Perhaps I need to do something else? Tomorrow I will check the threads to see if I stay on the same one or not. That part is a bit of a mystery to me. I need to understand this better.

Thanks, Murray

flowt-au commented 4 months ago

Final comment here: The test does all stay on the "Test runner thread" from the start of the test, to the reactive function, to the blocking function, back to the reactive, back to the test runner. So, looks like it is going to be ok, as far as I can see.