RedisGraph / redisgraph-py

RedisGraph python client
https://redisgraph.io
BSD 3-Clause "New" or "Revised" License
189 stars 49 forks source link

Let's improve graph.commit() performance using redis.pipeline() to sync state #82

Open boris-42 opened 4 years ago

boris-42 commented 4 years ago

Running commands one by one is slow as you have TCP overhead. Which can cause 10-100 performance degradation. To avoid it there are redis pipelines https://redis.io/topics/pipelining

Redis pipeline in python client willreturn all results as array of responses, so change in redisgraph should be minimal to support them. Here is the bottleneck place https://github.com/RedisGraph/redisgraph-py/blob/master/redisgraph/graph.py#L127-L128

Are you OK if Implement this?

gkorland commented 4 years ago

First, thanks for the suggestion to contribute the code! Second, you're right pipeline can improve performance dramatically and we already added this support in other clients. But, it can't replace the sync API but added as alternative API.

boris-42 commented 4 years ago

@gkorland First of all great that this is something on mind ;).

So you suggest to implement something like bulk_queries(queries) ?

Another thing is this place of code (as well please take a look at #81 )

    def commit(self):
        """
        Create entire graph.
        """
        if len(self.nodes) == 0 and len(self.edges) == 0:
            return None

        query = 'CREATE '
        for _, node in self.nodes.items():
            query += str(node) + ','

        query += ','.join([str(edge) for edge in self.edges])

        # Discard leading comma.
        if query[-1] is ',':
            query = query[:-1]

        return self.query(query)

The problem is that it only works for creation of things but not updates. As soon as we add functionality to update/delete we would switch to multiple Graph commands e.g.: MATCH + DELETE, MERGE + SET in such situation we would need to use multiple queries (that was sort of initial point, sorry for being unclear it was a bit late)