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.

Sign up for GitHub

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

Jump to bottom

Fix metadata sync issue for cache listeners in multi-tab #8343

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Fix metadata sync issue for cache listeners in multi-tab #8343

merged 3 commits into from
Sep 12, 2024

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Jun 28, 2024 •
edited
Loading

Issue: when creating a snapshot listener, it's QueryTargetState is simply set to "not-current" if it is listening to cache only. This is going to create out-of-sync metadata in persistence multi-tabs:

scenario:

  1. Create a snapshot listener in primary client, listen to server
  2. It raised a snapshot from server, which is up-to-date (fromCache=false), and store it into cache
  3. Listen to same query with source==cache in secondary tab
  4. Since the cached result is up-to-date, and the cache listener in the secondary tab is sharing the date with the server listener in the primary tab, the raised snapshot should be current as well (fromCache=false)

@milaGGL milaGGL requested review from a team as code owners June 28, 2024 19:10
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jun 28, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 6c3ef5b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Patch
firebase Patch
@firebase/firestore-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@DellaBitta
Copy link
Contributor

Could you please provide a PR description. A detail of the change, a link to the bug, etc? Thanks!

egilmorez
egilmorez reviewed Jun 28, 2024
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things, thanks!

We should also get Mark Arndt's review on all things Firestore.

packages/firestore/src/local/shared_client_state.ts Outdated Show resolved Hide resolved
.changeset/warm-oranges-itch.md Outdated Show resolved Hide resolved
@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • @firebase/app

    TypeBase (ecadbe3)Merge (9a793e5)Diff
    browser18.1 kB18.3 kB+141 B (+0.8%)
    esm523.7 kB23.8 kB+142 B (+0.6%)
    main24.8 kB24.9 kB+135 B (+0.5%)
    module18.1 kB18.3 kB+141 B (+0.8%)

  • @firebase/firestore

    TypeBase (ecadbe3)Merge (9a793e5)Diff
    browser378 kB378 kB+2 B (+0.0%)
    esm5363 kB363 kB+34 B (+0.0%)
    main582 kB582 kB+88 B (+0.0%)
    module378 kB378 kB+2 B (+0.0%)
    react-native378 kB378 kB+2 B (+0.0%)

  • @firebase/util

    TypeBase (ecadbe3)Merge (9a793e5)Diff
    browser23.1 kB23.2 kB+145 B (+0.6%)
    esm524.7 kB24.9 kB+145 B (+0.6%)
    main30.5 kB30.7 kB+219 B (+0.7%)
    module23.1 kB23.2 kB+145 B (+0.6%)

  • bundle

    TypeBase (ecadbe3)Merge (9a793e5)Diff
    firestore (CSI Auto Indexing Disable and Delete)268 kB268 kB+8 B (+0.0%)
    firestore (CSI Auto Indexing Enable)268 kB268 kB+8 B (+0.0%)
    firestore (Persistence)303 kB303 kB+16 B (+0.0%)
    firestore (Query Cursors)240 kB240 kB-6 B (-0.0%)
    firestore (Query)238 kB238 kB-6 B (-0.0%)
    firestore (Read data once)226 kB226 kB-6 B (-0.0%)
    firestore (Read Write w Persistence)322 kB322 kB+2 B (+0.0%)
    firestore (Realtime updates)228 kB228 kB-6 B (-0.0%)
    firestore (Transaction)205 kB205 kB+8 B (+0.0%)
    firestore (Write data)205 kB205 kB+8 B (+0.0%)

  • firebase

    TypeBase (ecadbe3)Merge (9a793e5)Diff
    firebase-app-compat.js31.4 kB31.7 kB+235 B (+0.7%)
    firebase-app.js102 kB102 kB+395 B (+0.4%)
    firebase-compat.js786 kB786 kB+251 B (+0.0%)
    firebase-firestore-compat.js341 kB341 kB+9 B (+0.0%)
    firebase-firestore.js437 kB437 kB+2 B (+0.0%)
    firebase-performance-standalone-compat.es2017.js93.3 kB93.5 kB+236 B (+0.3%)
    firebase-performance-standalone-compat.js70.4 kB70.6 kB+226 B (+0.3%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/m8DnXp6Xe1.html

@google-oss-bot
Copy link
Contributor

Size Analysis Report 1

Affected Products

  • @firebase/app

    • initializeServerApp

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size11.3 kB11.4 kB+141 B (+1.2%)
      size-with-ext-deps25.1 kB25.4 kB+236 B (+0.9%)

      External Dependency

      ModuleBase (ecadbe3)Merge (9a793e5)Diff
      @firebase/util

      ErrorFactory
FirebaseError
base64urlEncodeWithoutPadding
isBrowser
isIndexedDBAvailable
validateIndexedDBOpenable

      ErrorFactory
FirebaseError
base64urlEncodeWithoutPadding
isBrowser
isIndexedDBAvailable
isWebWorker
validateIndexedDBOpenable

      + isWebWorker

  • @firebase/firestore

    • addDoc

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size124 kB124 kB+8 B (+0.0%)
      size-with-ext-deps196 kB196 kB+8 B (+0.0%)

    • deleteAllPersistentCacheIndexes

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size87.6 kB87.6 kB+8 B (+0.0%)
      size-with-ext-deps159 kB159 kB+8 B (+0.0%)

    • deleteDoc

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size115 kB115 kB+8 B (+0.0%)
      size-with-ext-deps186 kB186 kB+8 B (+0.0%)

    • disableNetwork

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size103 kB103 kB+8 B (+0.0%)
      size-with-ext-deps175 kB175 kB+8 B (+0.0%)

    • disablePersistentCacheIndexAutoCreation

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size87.5 kB87.5 kB+8 B (+0.0%)
      size-with-ext-deps159 kB159 kB+8 B (+0.0%)

    • enableIndexedDbPersistence

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size186 kB186 kB+8 B (+0.0%)
      size-with-ext-deps258 kB258 kB+8 B (+0.0%)

    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size222 kB222 kB+16 B (+0.0%)
      size-with-ext-deps294 kB294 kB+16 B (+0.0%)

    • enableNetwork

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size103 kB103 kB+8 B (+0.0%)
      size-with-ext-deps175 kB175 kB+8 B (+0.0%)

    • enablePersistentCacheIndexAutoCreation

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size87.5 kB87.5 kB+8 B (+0.0%)
      size-with-ext-deps159 kB159 kB+8 B (+0.0%)

    • executeWrite

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size114 kB114 kB+8 B (+0.0%)
      size-with-ext-deps186 kB186 kB+8 B (+0.0%)

    • getAggregateFromServer

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size110 kB110 kB+8 B (+0.0%)
      size-with-ext-deps182 kB182 kB+8 B (+0.0%)

    • getCountFromServer

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size111 kB111 kB+8 B (+0.0%)
      size-with-ext-deps182 kB182 kB+8 B (+0.0%)

    • getDoc

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size145 kB145 kB-6 B (-0.0%)
      size-with-ext-deps216 kB216 kB-6 B (-0.0%)

    • getDocFromCache

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size95.0 kB95.0 kB+8 B (+0.0%)
      size-with-ext-deps166 kB166 kB+8 B (+0.0%)

    • getDocFromServer

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size145 kB145 kB-6 B (-0.0%)
      size-with-ext-deps216 kB216 kB-6 B (-0.0%)

    • getDocs

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size146 kB146 kB-6 B (-0.0%)
      size-with-ext-deps218 kB218 kB-6 B (-0.0%)

    • getDocsFromCache

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size103 kB103 kB+8 B (+0.0%)
      size-with-ext-deps174 kB174 kB+8 B (+0.0%)

    • getDocsFromServer

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size146 kB146 kB-6 B (-0.0%)
      size-with-ext-deps218 kB218 kB-6 B (-0.0%)

    • loadBundle

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size113 kB113 kB+8 B (+0.0%)
      size-with-ext-deps184 kB184 kB+8 B (+0.0%)

    • memoryEagerGarbageCollector

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size85.6 kB85.6 kB+8 B (+0.0%)
      size-with-ext-deps157 kB157 kB+8 B (+0.0%)

    • memoryLocalCache

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size100 kB100 kB+8 B (+0.0%)
      size-with-ext-deps172 kB172 kB+8 B (+0.0%)

    • memoryLruGarbageCollector

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size92.1 kB92.1 kB+8 B (+0.0%)
      size-with-ext-deps163 kB163 kB+8 B (+0.0%)

    • namedQuery

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size88.5 kB88.5 kB+8 B (+0.0%)
      size-with-ext-deps160 kB160 kB+8 B (+0.0%)

    • onSnapshot

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size147 kB147 kB-6 B (-0.0%)
      size-with-ext-deps219 kB219 kB-6 B (-0.0%)

    • onSnapshotsInSync

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size136 kB136 kB-6 B (-0.0%)
      size-with-ext-deps208 kB208 kB-6 B (-0.0%)

    • persistentLocalCache

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size184 kB184 kB+8 B (+0.0%)
      size-with-ext-deps255 kB255 kB+8 B (+0.0%)

    • persistentMultipleTabManager

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size219 kB219 kB+16 B (+0.0%)
      size-with-ext-deps291 kB291 kB+16 B (+0.0%)

    • persistentSingleTabManager

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size183 kB183 kB+8 B (+0.0%)
      size-with-ext-deps255 kB255 kB+8 B (+0.0%)

    • runTransaction

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size124 kB124 kB+8 B (+0.0%)
      size-with-ext-deps195 kB195 kB+8 B (+0.0%)

    • setDoc

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size123 kB123 kB+8 B (+0.0%)
      size-with-ext-deps195 kB195 kB+8 B (+0.0%)

    • setIndexConfiguration

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size91.1 kB91.1 kB+8 B (+0.0%)
      size-with-ext-deps162 kB162 kB+8 B (+0.0%)

    • updateDoc

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size124 kB124 kB+8 B (+0.0%)
      size-with-ext-deps195 kB195 kB+8 B (+0.0%)

    • waitForPendingWrites

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size104 kB104 kB+8 B (+0.0%)
      size-with-ext-deps175 kB175 kB+8 B (+0.0%)

    • writeBatch

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size126 kB126 kB+8 B (+0.0%)
      size-with-ext-deps197 kB197 kB+8 B (+0.0%)

  • @firebase/util

    • isBrowser

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size84 B197 B+113 B (+134.5%)
      size-with-ext-deps84 B197 B+113 B (+134.5%)

      Dependency

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      functions

      isBrowser

      isBrowser
isWebWorker

      + isWebWorker

    • isWebWorker

      Size

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      size?146 B? (?)
      size-with-ext-deps?146 B? (?)

      Dependency

      TypeBase (ecadbe3)Merge (9a793e5)Diff
      functions?

      isWebWorker

      		?
      classes??
      variables??
      enums??

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/A95xUfuXqC.html

@wu-hui wu-hui assigned milaGGL and unassigned wu-hui Jul 2, 2024
@milaGGL milaGGL requested a review from egilmorez July 2, 2024 18:00
@milaGGL milaGGL requested a review from markarndt July 24, 2024 17:06
@milaGGL milaGGL merged commit 629919e into main Sep 12, 2024
42 checks passed
@milaGGL milaGGL deleted the mila/fix-metadata-sync-issue-of-cache-listeners branch September 12, 2024 17:54
@google-oss-bot google-oss-bot mentioned this pull request Sep 16, 2024
Merged
dlarocque pushed a commit that referenced this pull request Sep 16, 2024
@milaGGL @dlarocque
Fix metadata sync issue for cache listeners in multi-tab (#8343)
79f38be
@WontonSam WontonSam mentioned this pull request Oct 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wu-hui wu-hui wu-hui approved these changes

@egilmorez egilmorez Awaiting requested review from egilmorez

@markarndt markarndt Awaiting requested review from markarndt

Assignees

@milaGGL milaGGL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@milaGGL @DellaBitta @google-oss-bot @egilmorez @wu-hui