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

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

Open
boris-42 opened this issue Jul 11, 2020 · 2 comments

Comments

@boris-42
Copy link
Contributor

boris-42 commented Jul 11, 2020

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
Copy link
Contributor

gkorland commented Jul 11, 2020

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
Copy link
Contributor Author

@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)

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