Skip to content

Feature/concurrent queries #5

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Zlyden-1
Copy link

Currently, concurrent queries raise pyneo4j_ogm.exceptions.TransactionInProgress: A transaction is already in progress., for example, in the following cases:

  1. asyncio.gather
async def test_concurrent_non_batch_execution(client: Pyneo4jClient, session: AsyncSession):
    coroutines_count = 3
    coroutines = []
    for i in range(coroutines_count):
        coroutines.append(
            client.cypher("CREATE (n:Node) SET n.name = $name", parameters={"name": f"TestName{i}"},)
        )
    await asyncio.gather(*coroutines)
  1. when different requests to api endpoints come up at the same time, and there's need to execute queries concurrently.

In this PR I made sort of workaroud, that includes:

  • single query creates and stores transaction inside client.cypher() method, that allows them to be ran concurrently
  • batch query can gets batch transaction from instance of class BatchManagerConcurrent, that is instantiated by client.concurrent_batch() method and passed into any method anything that runs a query in optional (and None by default) field batch_manager
  • for reverse compatibility old workflow is fully preserved

I look forward to using 1.0.0 version of this lib, and hope that you'll add features from this PR as minor improvement before major release and consider concepts from it for 1.0.0, alongside with Multithreading/Multiple clients support

@groc-prog
Copy link
Owner

I think this would be a good addition to the package. Looking at it now, I think we could make this a bit more smoother to work with by having each query have it's own transaction by default (so it would be scoped to that function call) and then only use a shared transaction for that client when batching queries together. So it would be something like:

class Client:
  _transaction: Optional[AsyncTransaction] = None

  async def cypher(self, ...):
    if self._transaction is not None:
      # Use this one since we are batching it at this point
      transaction = self._transaction
    else:
      # Get a new transaction
      transaction = self._begin_transation()

    # Do something with it
    
    if self._transaction is None:
      # Commit the transaction if not batching
      self._commit_transaction()

Of course this could be refined so we get rid of the currently duplicate code by having another class/method which handles transactions. But I have not used asyncio that much and I have not worked with Python professionally for about a year now, so I don't know from the top of my head if this could cause any other issues.

Regarding this feature, I don't plan on making any new releases before the 1.0.0 release, so this would be included in that release. There is already a branch for the 1.0.0 release here, so if you are ok with my proposal I would suggest creating a pull request with the changes there if you find some time. Since this already includes a client for Memgraph DB's, there would be a bit more to test since there are some differences in using the Neo4j driver package for these two DB's, but i think you can manage. If you find the time and have some questions about the local setup or new code structure, let me know and I will help you out where i can.

@groc-prog
Copy link
Owner

Hey, I got a update on this feature request. I implemented concurrent transactions like I described in my previous comment in this commit on the feature/next branch, there are also tests for it here. Could you take a look at it and get back to me whether this would fit your use case?

@groc-prog groc-prog added the v1.0.0 Planned to be part of 1.0.0 label Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.0.0 Planned to be part of 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants