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

firestore.recursiveDelete() doesn't work on collections with over 1000 documents #2292

Open
3 tasks done
mohatt opened this issue Feb 13, 2025 · 5 comments
Open
3 tasks done
Assignees
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. needs more info This issue needs more information from the customer to proceed. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mohatt
Copy link

mohatt commented Feb 13, 2025

  1. Is this a client library issue or a product issue?
    This is the client library for Firestore.

  2. Did someone already solve this?

  1. Do you have a support contract?
    No

Environment details

  • OS: macOS
  • Node.js version: 20
  • npm version: 11
  • @google-cloud/firestore version: 7.11.0

Steps to reproduce

  1. Create a firestore collection with over 1000 documents
  2. run firestore.recursiveDelete() on the collection reference
  3. The promise returned is never resolved and no docs are deleted
@mohatt mohatt added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Feb 13, 2025
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Feb 13, 2025
@mohatt mohatt changed the title firestore.recursiveDelete() doesn't work on collections with over 5000 documents firestore.recursiveDelete() doesn't work on collections with over 1000 documents Feb 13, 2025
@milaGGL
Copy link
Contributor

milaGGL commented Feb 13, 2025

Hi @mohatt , thank you for reporting this issue. could you please try:

  1. add a catch block and see it it captures anything
  2. set log level to "debug" and check the logs
  3. try recursiveDelete a smaller size of collection and see if it works as expected
  4. try deleting the same collection with Firebase CLI and see if it works as expected

@mohatt
Copy link
Author

mohatt commented Feb 13, 2025

Hi @milaGGL,

I tried the steps you mentioned:

  1. Add a catch block to see if an error is thrown
    I tried, but nothing is thrown since the promise is never resolved.
  2. Set log level to “debug” and check the logs
    There are no relevant logs that provide insight into the issue.
  3. Try recursiveDelete on a smaller collection
    It works for collections under 1000 documents (which matches the RECURSIVE_DELETE_MIN_PENDING_OPS constant in recursive-delete.ts).
  4. Try deleting the same collection using the Firebase CLI
    The Firebase CLI successfully deletes the collection, even when it has more than 1000 documents.

Potential Cause

After reviewing the source code, I believe the issue lies in the following condition in this line inside recursive-delete.ts:

if (streamedDocsCount < this.minPendingOps) { 
  this.onQueryEnd();

This condition is incorrect because this.maxPendingOps is actually being used as the limit for the query in this line:

query = query.select(FieldPath.documentId()).limit(this.maxPendingOps);

Since maxPendingOps determines how many documents should be fetched and processed at once, the check should be:

if (streamedDocsCount < this.maxPendingOps) { 

I believe this mismatch is causing the function to hang indefinitely when processing collections over this.minPendingOps (which is 1000 documents by default), as it is never able to meet the required condition to proceed with this.onQueryEnd() call and flush the pending BulkWriter operations and resolve the promise.

Let me know if you need any more details! Thanks.

@milaGGL milaGGL self-assigned this Feb 18, 2025
@milaGGL
Copy link
Contributor

milaGGL commented Feb 18, 2025

@mohatt, I have tried the following test and it worked as expected:

      const batch = firestore.batch();
      for(let i=0;i<1500; i++) {
        batch.set(randomCol.doc('doc' + i), {foo: 'bar'});
      }
      await batch.commit();

      let snap = await randomCol.get();
      expect(snap.size).to.equal(1500);

      await firestore.recursiveDelete(randomCol);

      snap = await randomCol.get();
      expect(snap.size).to.equal(0);

Would you be able to provide a minimal reproduction app?

@mohatt
Copy link
Author

mohatt commented Feb 22, 2025

Hey @milaGGL, your test case works fine, but after testing in multiple ways, I think that the issue only occurs when running in the local Firestore Emulator and only under certain conditions.

My use case is a bit more complex—it involves running firestore.recursiveDelete() on subcollections of certain documents while simultaneously performing other operations on those documents (using firestore.bulkWriter()). This happens as part of a query stream that processes many documents in batches.

I was able to fix the issue by modifying the condition from if (streamedDocsCount < this.minPendingOps) to if (streamedDocsCount < this.maxPendingOps). I’ll try to provide a reproduction case if I can isolate the problem further.

@milaGGL
Copy link
Contributor

milaGGL commented Feb 24, 2025

yes, a minimal repro app is appreciated. Thank you @mohatt.

@milaGGL milaGGL added the needs more info This issue needs more information from the customer to proceed. label Feb 24, 2025
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/nodejs-firestore API. needs more info This issue needs more information from the customer to proceed. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants