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

Ignore IndexedDB failures during garbage collection #3015

Merged
merged 3 commits into from
May 5, 2020

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented May 4, 2020

Rationale is that they the LRU task will retry its run when it is invoked next.

There are no tests since the GC scheduling code is not tested right now. Let me know if you would like me to add tests.

Note that there is a behavior change: If the primary lease is lost, the GC run is rescheduled. It will however get stoped in

Addresses #2755

Rationale is that they the LRU task will retry its run when it is invoked next.

There are no tests since the GC scheduling code is not tested right now. Let me know if you would like me to add tests.

Note that there is a behavior change: If the primary lease is lost, the GC run is rescheduled. It will however get stoped in
https://github.com/firebase/firebase-js-sdk/blob/5a60243bf264967819fc5c3897892084c5eb4d43/packages/firestore/src/core/component_provider.ts#L203
@schmidt-sebastian schmidt-sebastian changed the title Ignore IndexedDb failures during LRU Ignore IndexedDB failures during LRU May 4, 2020
@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 4, 2020

Binary Size Report

Affected SDKs

  • @firebase/firestore

    Type Base (da3582b) Head (0ee99d5) Diff
    browser 247 kB 248 kB +404 B (+0.2%)
    esm2017 193 kB 193 kB +111 B (+0.1%)
    main 488 kB 488 kB +442 B (+0.1%)
    module 245 kB 246 kB +382 B (+0.2%)
  • @firebase/firestore/memory

    Type Base (da3582b) Head (0ee99d5) Diff
    browser 188 kB 188 kB -35 B (-0.0%)
    esm2017 148 kB 148 kB -31 B (-0.0%)
    main 365 kB 365 kB -49 B (-0.0%)
    module 187 kB 187 kB -35 B (-0.0%)
  • firebase

    Type Base (da3582b) Head (0ee99d5) Diff
    firebase-firestore.js 286 kB 287 kB +380 B (+0.1%)
    firebase-firestore.memory.js 229 kB 228 kB -35 B (-0.0%)
    firebase.js 820 kB 820 kB +382 B (+0.0%)

Test Logs

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

await localStore.collectGarbage(this.garbageCollector);
} catch (e) {
if (e.name === 'IndexedDbTransactionError') {
logDebug(LOG_TAG, 'Ignoring IndexedDB error during LRU: ', e);
Copy link
Contributor

Choose a reason for hiding this comment

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

"LRU" is not a great descriptor of what's going on here. How about "garbage collection"?

Also update the PR description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@wilhuff wilhuff assigned schmidt-sebastian and unassigned wilhuff May 5, 2020
@schmidt-sebastian schmidt-sebastian changed the title Ignore IndexedDB failures during LRU Ignore IndexedDB failures during garbage collection May 5, 2020
@schmidt-sebastian schmidt-sebastian merged commit 76ba2ea into master May 5, 2020
@firebase firebase locked and limited conversation to collaborators Jun 5, 2020
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/dontcrashonlru branch November 9, 2020 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants