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

Support restart of FirestoreClient #8430

Merged
merged 13 commits into from
Sep 20, 2024

Conversation

tom-andersen
Copy link
Contributor

@tom-andersen tom-andersen commented Aug 16, 2024

This PR is purely a refactor, since restart functionality is not exposed in API surface, but simply prepares for future development of features.

Currently, the FirestoreClient becomes unusable when terminated. Certain functionality, such as clearing the cache, require the FirestoreClient be terminated. This presents a problem when Firestore backend requires clearing of cache on client, since this will break customer applications. This PR makes it possible to restart the client after terminate, thereby allowing usage of Firestore to continue even after cache is cleared.

To support restarting FirestoreClient:

  • Create new AsyncQueue that will start processing when old AsyncQueue has finished terminating. This is done by chaining old AsyncQueue terminate task to head of new AsyncQueue.
  • Since ComponentProvider retains copies of shutdown components, a new component provider is created upon restart. This requires a higher level of abstraction where we introduce a ComponentProviderFactory, such that we can create ComponentProvider of same shape/config multiple times.
  • Terminate will now clear reference to firestore client, such that Firestore will create a new instance through existing ensureFirestoreConfigured code path. To support existing behaviour of preventing Firestore usage after terminate, the ensureFirestoreConfigured method has guard added to check whether a _terminated task exists.
  • Restart is implemented by simply clearing _terminated task, such that ensureFirestoreConfigured no longer prevents instantiating new client after terminate. With both the reference to _firestoreClient and _termianted being reset to undefined, the system appears fresh, ready to create a new firestore instance. By having the old AsyncQueue chained to new AsyncQueue, no processing will happen on new firestore instance, until all processing on old firestore instance is complete, thereby avoiding any interference between the old and new firestore instance.

Copy link

changeset-bot bot commented Aug 16, 2024

🦋 Changeset detected

Latest commit: 8eaee68

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 16, 2024

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (629919e)Merge (02f1e4f)Diff
    browser381 kB381 kB-331 B (-0.1%)
    esm5366 kB366 kB+73 B (+0.0%)
    main587 kB587 kB+269 B (+0.0%)
    module381 kB381 kB-331 B (-0.1%)
    react-native382 kB381 kB-330 B (-0.1%)
  • @firebase/firestore-lite

    TypeBase (629919e)Merge (02f1e4f)Diff
    browser111 kB111 kB+179 B (+0.2%)
    esm5108 kB108 kB+399 B (+0.4%)
    main153 kB153 kB+193 B (+0.1%)
    module111 kB111 kB+179 B (+0.2%)
    react-native111 kB111 kB+179 B (+0.2%)
  • bundle

    15 size changes

    TypeBase (629919e)Merge (02f1e4f)Diff
    firestore (CSI Auto Indexing Disable and Delete)270 kB269 kB-664 B (-0.2%)
    firestore (CSI Auto Indexing Enable)270 kB269 kB-664 B (-0.2%)
    firestore (Persistence)305 kB303 kB-1.84 kB (-0.6%)
    firestore (Query Cursors)242 kB242 kB+268 B (+0.1%)
    firestore (Query)240 kB240 kB+268 B (+0.1%)
    firestore (Read data once)228 kB228 kB+268 B (+0.1%)
    firestore (Read Write w Persistence)325 kB325 kB-33 B (-0.0%)
    firestore (Realtime updates)230 kB230 kB+261 B (+0.1%)
    firestore (Transaction)207 kB207 kB+247 B (+0.1%)
    firestore (Write data)207 kB207 kB+247 B (+0.1%)
    firestore-lite (Query Cursors)91.3 kB91.4 kB+176 B (+0.2%)
    firestore-lite (Query)87.4 kB87.6 kB+176 B (+0.2%)
    firestore-lite (Read data once)62.9 kB63.1 kB+176 B (+0.3%)
    firestore-lite (Transaction)88.1 kB88.3 kB+179 B (+0.2%)
    firestore-lite (Write data)72.5 kB72.6 kB+176 B (+0.2%)

  • firebase

    TypeBase (629919e)Merge (02f1e4f)Diff
    firebase-compat.js788 kB788 kB-129 B (-0.0%)
    firebase-firestore-compat.js344 kB344 kB-127 B (-0.0%)
    firebase-firestore-lite.js119 kB119 kB+179 B (+0.2%)
    firebase-firestore.js440 kB440 kB-294 B (-0.1%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 16, 2024

Size Analysis Report 1

This report is too large (967,691 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

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

@tom-andersen tom-andersen marked this pull request as ready for review September 15, 2024 17:38
@tom-andersen tom-andersen requested review from a team as code owners September 15, 2024 17:38
@tom-andersen tom-andersen requested a review from wu-hui September 15, 2024 17:38
Copy link
Contributor

@DellaBitta DellaBitta left a comment

Choose a reason for hiding this comment

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

Please add a changeset and a PR description based on the template. Thanks!

@dconeybe
Copy link
Contributor

Drive-by comment: Could you add a blurb to this PR description to explain why this change is being made? Is it fixing a bug? Is it setting the stage for a new feature? Is it a performance improvement? Thanks.

@wu-hui wu-hui assigned tom-andersen and unassigned wu-hui Sep 18, 2024
@tom-andersen tom-andersen removed their assignment Sep 18, 2024
@tom-andersen tom-andersen merged commit 47b0913 into main Sep 20, 2024
43 checks passed
@tom-andersen tom-andersen deleted the tomandersen/restartFirestoreClient branch September 20, 2024 19:29
@google-oss-bot google-oss-bot mentioned this pull request Sep 30, 2024
@firebase firebase locked and limited conversation to collaborators Oct 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants