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

Dedupe sync/async paths of recursive delete code #425

Closed
crwilcox opened this issue Aug 16, 2021 · 1 comment
Closed

Dedupe sync/async paths of recursive delete code #425

crwilcox opened this issue Aug 16, 2021 · 1 comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. type: cleanup An internal cleanup or hygiene concern.

Comments

@crwilcox
Copy link
Contributor

https://github.com/googleapis/python-firestore/pull/420/files#diff-ae4779c3bae12c581abba3732b4ed80ab155b08787b617798a06e36cb0642052R342

Currently the sync/async versions of recursive delete are pretty similar. A thought: What if the sync version managed accessing the event loop, using the async implementation, and blocking.

Perf should likely be investigated when doing this but it may be a way to avoid duplicate code throughout the two halves if the event loop doesn't add significant overhead.

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Aug 16, 2021
@kolea2 kolea2 added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Aug 19, 2021
@meredithslota meredithslota added type: cleanup An internal cleanup or hygiene concern. and removed type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Aug 16, 2023
@daniel-sanche
Copy link
Contributor

daniel-sanche commented Sep 18, 2023

We investigated something similar for bigtable, but came across this forum thread:

async code is written under the assumption that the sync code it calls never blocks for I/O, and definitely never enters the event loop recursively. Only await may enter the event loop. This is used in many places to avoid explicit locks: async code can rely on the fact that no other code runs concurrently unless explicitly allowed by await.

It seems that calling into the event loop through the sync function is not supported and can result in deadlock. We did some experimenting with sync code generation from an async implementation, and that might be a safer way to avoid maintaining duplicate code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

4 participants