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

Hanging query follow up #7771

Merged
merged 13 commits into from
Nov 17, 2023
Merged

Hanging query follow up #7771

merged 13 commits into from
Nov 17, 2023

Conversation

MarkDuckworth
Copy link
Contributor

@MarkDuckworth MarkDuckworth commented Nov 10, 2023

AIs for hanging query issue.

  • Update WebChannel to include the fix.
  • Integration tests
  • Enable useFetchStreams
  • Add hardAssert to detect the hung state.

Implements b/308456881 and b/308456686
Fixes #6118

Copy link

changeset-bot bot commented Nov 10, 2023

🦋 Changeset detected

Latest commit: 3954ea2

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

This PR includes changesets to release 4 packages
Name Type
@firebase/firestore Patch
@firebase/webchannel-wrapper 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 Nov 10, 2023

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (bebecda)Merge (e3ae621)Diff
    browser374 kB374 kB-13 B (-0.0%)
    esm5360 kB359 kB-33 B (-0.0%)
    main575 kB576 kB+37 B (+0.0%)
    module374 kB374 kB-13 B (-0.0%)
    react-native374 kB374 kB-14 B (-0.0%)
  • @firebase/webchannel-wrapper

    TypeBase (bebecda)Merge (e3ae621)Diff
    esm555.4 kB55.4 kB-6 B (-0.0%)
    main65.4 kB65.4 kB-6 B (-0.0%)
    module53.1 kB53.1 kB+2 B (+0.0%)
  • bundle

    TypeBase (bebecda)Merge (e3ae621)Diff
    firestore (Transaction)204 kB204 kB-15 B (-0.0%)
    firestore (Write data)204 kB204 kB-15 B (-0.0%)
  • firebase

    TypeBase (bebecda)Merge (e3ae621)Diff
    firebase-compat.js778 kB778 kB+4 B (+0.0%)
    firebase-firestore-compat.js340 kB340 kB+4 B (+0.0%)
    firebase-firestore.js434 kB434 kB+44 B (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 10, 2023

Size Analysis Report 1

This report is too large (189,632 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/hGkiCiXAKl.html

@MarkDuckworth MarkDuckworth marked this pull request as ready for review November 13, 2023 22:39
@MarkDuckworth MarkDuckworth requested review from a team as code owners November 13, 2023 22:39
@MarkDuckworth MarkDuckworth changed the title Markduckworth/ais hanging query Hanging query follow up Nov 13, 2023
@dconeybe
Copy link
Contributor

@MarkDuckworth Is there a GitHub Issue or Buganizer ticket that is canonically tracking the bug that this PR fixes?

@MarkDuckworth MarkDuckworth merged commit 00235ba into master Nov 17, 2023
41 checks passed
@MarkDuckworth MarkDuckworth deleted the markduckworth/ais-hanging-query branch November 17, 2023 22:58
@tgangso
Copy link

tgangso commented Nov 20, 2023

Any ETA on release? Thanks

@dconeybe
Copy link
Contributor

@tgangso We don't typically make any promises about release dates, but I'd expect this release before the end of the month (i.e. by Nov 30, 2023).

@tgangso
Copy link

tgangso commented Nov 22, 2023

I noticed the comment in #6118 mentioning this does not fix the issue completely. Commenting is closed in that case, so I will add a comment here. Been stuck on firebase v8 for very long now because of this issue, and just want to comment that version 8 does not have this issue, if that helps. Any version 9 or newer has the issue.

@thomasdao
Copy link

I have tested the latest version 10.7.0 with this fix and unfortunately the issue #7652 still happens and the query still hang. @MarkDuckworth

@MarkDuckworth
Copy link
Contributor Author

@tgangso, There was a false negative when validating the fix for #6118 in 10.7.0. That lead to the comment you are referring to. After additional review, the testing shows that the memory issue is fixed in 10.7.0. See the updated comment #6118 (comment)

@MarkDuckworth
Copy link
Contributor Author

@thomasdao, I'm no longer able to run the reproduction you initially provided. Can you enable debug logging for the SDK (setLogLevel('debug');) and provide us with the logs from the SDK when the issue is occurring. The first thing we will want to do is verify the debug log is printing SDK version 10.7.0 or higher. We can also use the log to look for the signature of the hanging query issue.

I recommend reopening your issue #7652 if you believe the issue is not fixed. Logs can send in a comment, gist, or with a Firebase Support Case.

@thomasdao
Copy link

Hi @MarkDuckworth, I thought you have cloned the data so I disabled the access to my Firestore database. I have re-enabled the access, please try again. When I tried with the new version 10.7.0, I saw the error below in the console, and the query still hangs. The issue #7652 on Github has been locked and I can't reopen from my side.

@firebase/firestore: Firestore (10.7.0): WebChannelConnection RPC 'Listen' stream 0x5b9a037f transport errored: Wn {type: 'c', target: Hn, g: Hn, defaultPrevented: false, status: 1}

@MarkDuckworth
Copy link
Contributor Author

@thomasdao,

Thanks for re-enabling access for your test project. I ran the code today and saw the error message you reported followed by a message with the error code unavailable.

203.js:2 [2023-12-07T21:43:15.567Z]  @firebase/firestore: Firestore (10.7.0): WebChannelConnection RPC 'Listen' stream 0x6eb9c2c0 transport errored: Wn {type: 'c', target: Hn, g: Hn, defaultPrevented: false, status: 1}
203.js:2 [2023-12-07T21:43:15.569Z]  @firebase/firestore: Firestore (10.7.0): PersistentStream close with error: FirebaseError: [code=unavailable]: The operation could not be completed

Code unavailable is a GRPC status from the backend that can be retried. The times I saw this error in your repro the SDK did retry and results were received after a delay. Are you seeing the same?

@thomasdao
Copy link

Hi @MarkDuckworth, I run the test project again and still see the error WebChannelConnection RPC 'Listen' stream 0x6eb9c2c0 transport errored.

I did not see the error PersistentStream close with error: FirebaseError: [code=unavailable]. I've waited for more than 10 minutes and the query still can't complete.

When I changed to the older version of Firebase (10.6.0), the query completes quickly.

@MarkDuckworth
Copy link
Contributor Author

@thomasdao, can you open a new bug? From what I can tell from the log message you provided, this is not the same issue as the hanging query issue that was fixed in 10.5.2. We made your issue #7652 the primary report for that issue because you provided a reproduction.

Given that we're currently not seeing the same behavior as you on your repro with 10.7.0, can you provide complete SDK logs from your application during your reproduction? Those can be sent securely by contacting support at https://firebase.google.com/support/troubleshooter/contact

@firebase firebase locked and limited conversation to collaborators Dec 18, 2023
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.

Firebase is keep crashing because of OOM on mobile Safari
9 participants