Skip to content
This repository was archived by the owner on Dec 25, 2023. It is now read-only.
This repository was archived by the owner on Dec 25, 2023. It is now read-only.

Various NDB Model hooks sometimes fail if they contain synchronous NDB RPC calls and are used within tasklets/async code #274

@aetherknight

Description

@aetherknight

At my current company, we often use NDB Model's _pre_put_hook and _pre_delete_hook to perform data validations that involve querying datastore. Most of the time, this works fine. However, sometimes when the hooks are run within sufficiently complicated/parallel asynchronous/tasklet code with a large number of coroutines, NDB's event loop code will raise a RuntimeError: maximum recursion depth exceeded while calling a Python object:

class SomeModel(ndb.Model):

    def _pre_put_hook(self):
        type(self).query().fetch()

@ndb.tasklet
def some_tasklet():
    model = SomeModel()
    yield model.put_async()
    raise ndb.Return()

@ndb.tasklet
def another_tasklet():
    yield some_tasklet()
    raise ndb.Return()

futs = [another_tasklet() for _ in xrange(100)]  # If I lower this to 50 then the error doesn't occur

[fut.get_result() for fut in futs]

My understanding of writing NDB tasklets and how the event loop works makes it pretty clear that one needs to be careful to not call synchronous IO/RPC calls. However, it is unclear from the NDB documentation that the event hooks are normal non-tasklet methods that are usually run in an asynchronous context (they are called by various async methods, such as Model.put_async usually before calling code that returns a Future, so the hook could be executed when called from either synchronous code or from within another tasklet).

Would it be possible to either:

  • Document this fact and make it clear that the NDB hooks should never be used to perform other RPC calls if you make use of NDB tasklets. This loses the benefit of performing more-complex data-based validations within NDB itself, however.
  • Add support for NDB Model hooks that have been decorated to become ndb.tasklets (so you could either have non-tasklet hooks or tasklet hooks), eg by checking to see if it returns a Future or not. This would probably also require some additional documentation to explain the feature.

I think the latter is a reasonable change because it would not break existing NDB code, and I'm willing to provide a PR once my company sorts out the CLA.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions