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

Collection snapshot queries with no documents don't store anything in cache. Always requires server snapshot. #5873

Closed
burtonator opened this issue Jan 7, 2022 · 6 comments · Fixed by #6624, firebase/firebase-android-sdk#4207 or firebase/firebase-ios-sdk#10437
Assignees

Comments

@burtonator
Copy link

[REQUIRED] Describe your environment

  • Operating System version: MacOS 12.1
  • Browser version: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.71 Safari/537.36
  • Firebase SDK version: 8.10.0
  • Firebase Product: firestore

[REQUIRED] Describe the problem

If a collection snapshot query doesn't have any documents, nothing is stored in the cache.

Each time we try to wait for an empty collection it has to wait for a server snapshot.

That should be a quick fix.

It's either not from cache or fromCache in the metadata is wrong.

Here's our code:

https://gist.github.com/burtonator/b9535237fd1c726960fb0fc77430160a

Maybe what's happening is that since no documents at ALL are found, the code assumes nothing is in the cache.

You should store a negative cache entry for that use case.

This is SUPER important for us because the latency is high when waiting for a server snapshot and this breaks our offline support.

Steps to reproduce:

Relevant Code:

Pseudocode is here:

https://gist.github.com/burtonator/b9535237fd1c726960fb0fc77430160a

@dconeybe
Copy link
Contributor

@burtonator thank you for reporting this issue. I'll look into a way to fix it.

@dconeybe
Copy link
Contributor

Just a heads up that I like to document my findings and progress as I go along so feel free to ignore my status updates :)

Here is the line where the empty snapshot is not raised:

// Raise data from cache if we have any documents or we are offline
return !snap.docs.isEmpty() || onlineState === OnlineState.Offline;

Currently a snapshot with no documents could either mean that the cached query contained no documents or there is no cache for the query. We need to add a way to distinguish between these two cases to fix this.

@burtonator
Copy link
Author

What is normally done is to store a 'negative cache' entry meaning we did fetch the results, but none were found, so you store this negative cache entry then, when you fetch again you emit the empty /null result.

Thanks for the updates! Appreciate it!

@dconeybe
Copy link
Contributor

Just a quick update. I'm working on the fix for this but it is a low priority relative to other work that I have on my plate. At the moment I have a hacky, proof-of-concept solution with some broken edge cases that needs to be cleaned up and have improved test coverage: #5897. You can watch that PR to see the progress but I don't have an ETA for getting the fix merged.

@dconeybe
Copy link
Contributor

dconeybe commented Oct 7, 2022

Update: My colleague has merged a fix for this bug: #6624. It will be included in the next release, which is scheduled for mid-October 2022. The fix will also be ported to Android and iOS (but no ETA for those fixes).

@dconeybe
Copy link
Contributor

FYI the fix was released in version 9.12.0 on October 11, 2022: https://firebase.google.com/support/release-notes/js#version_9120_-_october_11_2022

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.