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 batch deletes emit a snapshot per delete #13883

Open
sjudd opened this issue Oct 12, 2024 · 9 comments
Open

Firestore batch deletes emit a snapshot per delete #13883

sjudd opened this issue Oct 12, 2024 · 9 comments

Comments

@sjudd
Copy link

sjudd commented Oct 12, 2024

Description

Batch deletes cause the firestore ios client to emit a snapshot for every individual delete, which creates an exponential (edit: N + N-1 + N-2, triangular?) amount work decoding every document in each snapshot.

I would expect to get a single snapshot for the single batch delete. Firebase's documentation says "A batch of writes completes atomically and can write to multiple documents". The documentation seems to recommend batch writes over transactions for cases where you're not reading: "Like transactions, batched writes are atomic. Unlike transactions, batched writes do not need to ensure that read documents remain un-modified which leads to fewer failure cases."

I'd also expect that if multiple changes happen in the background, the next time the ios client connects, it emits a single snapshot with that complete set of changes, not one snapshot per change.

Reproducing the issue

Write 500 documents to a Firestore collection using Firestore's batch API with an admin SDK.
Observe the collection in a snapshot listener on iOS.
Receive a single snapshot update from network with 500 documents (WAI)

Then, using the same batch API, delete all 500 visits, roughly:

    all_documents = list(collection.list_documents())
    batch = db.batch()
    for doc in all_documents:
        batch.delete(doc)
    batch.commit()

On the client, add a snapshot listener to the collection:

let options = SnapshotListenOptions().withIncludeMetadataChanges(includeMetadataChanges)
db.collectionGroup("collection")
    .whereField(...)
    .order(by: "soc_date", descending: true)
    .addSnapshotListener(options: options) { querySnapshot, error in
       Log("Got snapshot, from cache: \(snapshot?.metadata.isFromCache ?? "nil"));
    }

Receive separate snapshots marked "from cache" for every single delete:

Got snapshot, from cache: 1, documents: 500
Got snapshot, from cache: 1, documents: 499
Got snapshot, from cache: 1, documents: 498
Got snapshot, from cache: 1, documents: 497
Got snapshot, from cache: 1, documents: 496
...

If you do this with a collection of 5000 items and you decode every item in each snapshot on the client, you end up decoding ~12 million documents during this process (5000 + 4999 + 4998 etc). Firebase also seems to emit the snapshots quite slowly (< 1 per second at the start). This is pretty catastrophic for app performance and users' battery life.

I also see this behavior for Android FWIW.

Firebase SDK Version

10.24.0

Xcode Version

15.0.1

Installation Method

Swift Package Manager

Firebase Product(s)

Firestore

Targeted Platforms

iOS

Relevant Log Output

No response

If using Swift Package Manager, the project's Package.resolved

Expand Package.resolved snippet
Replace this line with the contents of your Package.resolved.

If using CocoaPods, the project's Podfile.lock

Expand Podfile.lock snippet
Replace this line with the contents of your Podfile.lock!
@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@sjudd
Copy link
Author

sjudd commented Oct 14, 2024

I think either this or something similar was previously reported here: #5955. To respond to #5955 (comment):

Using isFromCache to exclude snapshots is not practical if you ever want to be able to read from cache. There's nothing that I can find that would allow you to tell the difference between these spurious updates and a normal read from cache when the client doesn't have a network connection.

Even if I do nothing in my listener, the CPU usage and heat generated by firestore are high. My app is also killed in the background when it would otherwise be allowed to run, presumably due to the high resource usage.

@MarkDuckworth
Copy link
Contributor

@sjudd, I'm unable to reproduce this issue. Version 10.24.0 is about 6 months old, can you update to the latest version and confirm the issue still exists before we investigate further?

If still reproducing with the latest SDK, can you provide logs from the SDK or an MCVE?

@MarkDuckworth
Copy link
Contributor

Also, what server SDK and version are you using?

@sjudd
Copy link
Author

sjudd commented Oct 15, 2024

I've updated the iOS client to 10.29.0 and can still reproduce using the steps I listed above 100% of the time. Ditto for Android on version 33.4.0. I'm using 6.5.0 of the firebase admin sdk.

Here's the specific query I'm using:

    let db = Firestore.firestore()
    db.collectionGroup("collection")
      .whereField("identifier", in: ["myIdentifier"])
      .whereField("date", isGreaterThanOrEqualTo: twoWeeksAgo)
      .whereField("date", isLessThan: twoWeeksFromNow)
      .order(by: "date", descending: true)
      .limit(to: 1000)
      .addSnapshotListener(options: options) { querySnapshot, error in
        guard let snapshot = querySnapshot else {
          DDLogError("Error fetching results: \(error)")
          return
        }
        DDLogInfo(
          """
          Got snapshot from firebase: \(snapshot.count) - fromCache: \
          \(snapshot.metadata.isFromCache)
          """
        )
      }

I'd be happy to provide sdk logs if you can tell me how. I do not have time to extract a demo app unfortunately.

@MarkDuckworth
Copy link
Contributor

@sjudd,
You can enable logging for the SDK with the instructions here: https://gist.github.com/katowulf/0475fb7a5907ed757f687aab6ed15878

To securely share a log, one way is to submit a firebase support case and attach the generate log in a file. Please reference internal bug b/373374534, so the team can route the log to the correct place. Thanks!

@sjudd
Copy link
Author

sjudd commented Oct 20, 2024

I've sent a support case and referenced the bug you mentioned: 10314628

@MarkDuckworth
Copy link
Contributor

@sjudd, I was not able to get your logs. Can you confirm the status of the support case you opened to share the logs? Also, confirm that your referenced the correct bug in your support request: "b/373374534".

@sjudd
Copy link
Author

sjudd commented Nov 5, 2024

I received an email on 10/22 indicating that the case was received. The title is Subject: b/3733745 Firestore iOS and Android emit snaphots per delete from batch deletes. I think that's the bug you mentioned in the Android thread. I'll ask them to include this bug as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants