Skip to content

Conversation

tusharkhandelwal8
Copy link
Contributor

This PR focuses on enhancing the reliability and efficiency of the Remote Config real-time stream, particularly concerning background app states. We've refined the ConfigRealtimeHttpClient to retrieve and manage the underlying input and error streams more effectively, ensuring proper closure even in error scenarios. Additionally, the closeRealtimeHttpStream method now explicitly closes these streams using the acquired references.

A key improvement is the robust handling of the app's background state. The setRealtimeBackgroundState method is now synchronized to guarantee thread-safe updates and will immediately close the real-time HTTP connection when the app transitions to the background. This background state is also reflected in ConfigAutoFetch, allowing for conditional exception logging during real-time stream listening, ensuring logs are only generated when the app is actively in the foreground. Furthermore, ConfigAutoFetch now maintains a dedicated InputStream reference to ensure proper stream closure, and a separate reference is kept in beginRealtimeHttpStream to handle potential early closure scenarios before ConfigAutoFetch is reached.

Copy link
Contributor

github-actions bot commented Mar 28, 2025

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 28, 2025

Coverage Report 1

Affected Products

  • firebase-config

    Overall coverage changed from ? (58b9da4) to 84.30% (2b6cb5b) by ?.

    34 individual files with coverage change

    FilenameBase (58b9da4)Merge (2b6cb5b)Diff
    AutoValue_ConfigUpdate.java?29.41%?
    Code.java?0.00%?
    ConfigAutoFetch.java?86.73%?
    ConfigCacheClient.java?93.33%?
    ConfigContainer.java?94.29%?
    ConfigFetchHandler.java?92.94%?
    ConfigFetchHttpClient.java?86.27%?
    ConfigGetParameterHandler.java?96.45%?
    ConfigRealtimeHandler.java?41.38%?
    ConfigRealtimeHttpClient.java?73.45%?
    ConfigSharedPrefsClient.java?87.50%?
    ConfigStorageClient.java?100.00%?
    ConfigUpdate.java?100.00%?
    ConfigUpdateListener.java?0.00%?
    ConfigUpdateListenerRegistration.java?0.00%?
    CustomSignals.java?100.00%?
    DefaultsXmlParser.java?0.00%?
    FirebaseRemoteConfig.java?89.76%?
    FirebaseRemoteConfigClientException.java?75.00%?
    FirebaseRemoteConfigException.java?95.65%?
    FirebaseRemoteConfigFetchThrottledException.java?100.00%?
    FirebaseRemoteConfigInfo.java?0.00%?
    FirebaseRemoteConfigInfoImpl.java?100.00%?
    FirebaseRemoteConfigServerException.java?68.42%?
    FirebaseRemoteConfigSettings.java?61.54%?
    FirebaseRemoteConfigValue.java?0.00%?
    FirebaseRemoteConfigValueImpl.java?84.62%?
    Personalization.java?91.43%?
    RemoteConfig.kt?31.58%?
    RemoteConfigComponent.java?90.70%?
    RemoteConfigConstants.java?0.00%?
    RemoteConfigRegistrar.java?100.00%?
    RolloutsStateFactory.java?95.24%?
    RolloutsStateSubscriptionsHandler.java?100.00%?

Test Logs

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

Copy link
Contributor

github-actions bot commented Mar 28, 2025

Test Results

 42 files   -    992   42 suites   - 992   1m 23s ⏱️ - 32m 55s
324 tests  -  5 548  324 ✅  -  5 526  0 💤  - 22  0 ❌ ±0 
660 runs   - 11 147  660 ✅  - 11 103  0 💤  - 44  0 ❌ ±0 

Results for commit 3c879c7. ± Comparison against base commit 58b9da4.

This pull request removes 5550 and adds 2 tests. Note that renamed tests count towards both.
com.google.android.datatransport.cct.CctBackendFactoryTest ‑ create_returnCCTBackend_WhenBackendNameIsCCT
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldOnlySupportProtoAndJson
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldSupportProtoAndJson
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOffline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOnline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldAddCookieOnPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldDropCookieOnMixedPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_CompressedResponseIsUncompressed
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirectsMoreThan5Times_shouldOnlyRedirect4Times
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirects_shouldCorrectlyFollowTheRedirectViaPost
…
com.google.firebase.remoteconfig.FirebaseRemoteConfigTest ‑ realtime_stream_listen_backgrounded_disconnects
com.google.firebase.remoteconfig.FirebaseRemoteConfigTest ‑ realtime_stream_listen_get_inputstream_exception_handling

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 28, 2025

Size Report 1

Affected Products

  • firebase-config

    TypeBase (58b9da4)Merge (2b6cb5b)Diff
    aar112 kB113 kB+639 B (+0.6%)
    apk (aggressive)211 kB212 kB+492 B (+0.2%)
    apk (release)4.59 MB4.59 MB+740 B (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 8, 2025

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • fire-rc

    DeviceStatisticsDistributions
    oriole-32
    Percentile58b9da42b6cb5bDiffSignificant (?)
    p1069.2 ±9 μs350 ±368 μs+281 μs (+406.0%)MAYBE
    p2580.2 ±21 μs364 ±376 μs+284 μs (+353.4%)MAYBE
    p5097.9 ±24 μs394 ±391 μs+296 μs (+302.3%)MAYBE
    p75125 ±32 μs459 ±422 μs+333 μs (+266.2%)NO
    p90188 ±43 μs593 ±530 μs+405 μs (+215.0%)NO

    20 test runs in comparison
    CommitTest Runs
    58b9da4
    • 2025-04-09_15:36:07.306830_Qqzs
    • 2025-04-09_15:36:07.306869_CaiJ
    • 2025-04-09_15:36:07.306879_KHBY
    • 2025-04-09_15:36:07.306888_uxgf
    • 2025-04-09_15:36:07.306895_vlYG
    • 2025-04-09_15:36:07.306901_lrMv
    • 2025-04-09_15:36:07.306908_Jrnt
    • 2025-04-09_15:36:07.306914_OqXO
    • 2025-04-09_15:36:07.306920_mPdN
    • 2025-04-09_15:36:07.306927_Lsdh
    2b6cb5b
    • 2025-04-09_16:06:07.568298_eqBw
    • 2025-04-09_16:06:07.568336_Xjll
    • 2025-04-09_16:06:07.568346_eNrM
    • 2025-04-09_16:06:07.568354_xyUn
    • 2025-04-09_16:06:07.568362_rlSf
    • 2025-04-09_16:06:07.568369_AWdl
    • 2025-04-09_16:06:07.568375_xIvI
    • 2025-04-09_16:06:07.568382_lqiN
    • 2025-04-09_16:06:07.568388_LSFO
    • 2025-04-09_16:06:07.568395_eNel
    redfin-30
    Percentile58b9da42b6cb5bDiffSignificant (?)
    p10402 ±455 μs253 ±220 μs-149 μs (-37.1%)NO
    p25468 ±523 μs286 ±262 μs-182 μs (-38.9%)NO
    p50592 ±685 μs371 ±393 μs-221 μs (-37.3%)NO
    p75800 ±859 μs506 ±558 μs-294 μs (-36.7%)NO
    p901.18 ±1 ms803 ±866 μs-382 μs (-32.2%)NO

    20 test runs in comparison
    CommitTest Runs
    58b9da4
    • 2025-04-09_15:36:07.306830_Qqzs
    • 2025-04-09_15:36:07.306869_CaiJ
    • 2025-04-09_15:36:07.306879_KHBY
    • 2025-04-09_15:36:07.306888_uxgf
    • 2025-04-09_15:36:07.306895_vlYG
    • 2025-04-09_15:36:07.306901_lrMv
    • 2025-04-09_15:36:07.306908_Jrnt
    • 2025-04-09_15:36:07.306914_OqXO
    • 2025-04-09_15:36:07.306920_mPdN
    • 2025-04-09_15:36:07.306927_Lsdh
    2b6cb5b
    • 2025-04-09_16:06:07.568298_eqBw
    • 2025-04-09_16:06:07.568336_Xjll
    • 2025-04-09_16:06:07.568346_eNrM
    • 2025-04-09_16:06:07.568354_xyUn
    • 2025-04-09_16:06:07.568362_rlSf
    • 2025-04-09_16:06:07.568369_AWdl
    • 2025-04-09_16:06:07.568375_xIvI
    • 2025-04-09_16:06:07.568382_lqiN
    • 2025-04-09_16:06:07.568388_LSFO
    • 2025-04-09_16:06:07.568395_eNel
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentile58b9da42b6cb5bDiffSignificant (?)
    p10199 ±5 ms200 ±3 ms+634 μs (+0.3%)NO
    p25204 ±4 ms206 ±3 ms+1.46 ms (+0.7%)NO
    p50211 ±5 ms213 ±3 ms+2.04 ms (+1.0%)NO
    p75219 ±6 ms221 ±4 ms+2.13 ms (+1.0%)NO
    p90227 ±7 ms231 ±7 ms+3.61 ms (+1.6%)NO

    20 test runs in comparison
    CommitTest Runs
    58b9da4
    • 2025-04-09_15:36:07.306830_Qqzs
    • 2025-04-09_15:36:07.306869_CaiJ
    • 2025-04-09_15:36:07.306879_KHBY
    • 2025-04-09_15:36:07.306888_uxgf
    • 2025-04-09_15:36:07.306895_vlYG
    • 2025-04-09_15:36:07.306901_lrMv
    • 2025-04-09_15:36:07.306908_Jrnt
    • 2025-04-09_15:36:07.306914_OqXO
    • 2025-04-09_15:36:07.306920_mPdN
    • 2025-04-09_15:36:07.306927_Lsdh
    2b6cb5b
    • 2025-04-09_16:06:07.568298_eqBw
    • 2025-04-09_16:06:07.568336_Xjll
    • 2025-04-09_16:06:07.568346_eNrM
    • 2025-04-09_16:06:07.568354_xyUn
    • 2025-04-09_16:06:07.568362_rlSf
    • 2025-04-09_16:06:07.568369_AWdl
    • 2025-04-09_16:06:07.568375_xIvI
    • 2025-04-09_16:06:07.568382_lqiN
    • 2025-04-09_16:06:07.568388_LSFO
    • 2025-04-09_16:06:07.568395_eNel
    redfin-30
    Percentile58b9da42b6cb5bDiffSignificant (?)
    p10226 ±6 ms245 ±4 ms+19.4 ms (+8.6%)MAYBE
    p25233 ±5 ms252 ±4 ms+19.4 ms (+8.4%)MAYBE
    p50240 ±6 ms260 ±4 ms+19.5 ms (+8.1%)MAYBE
    p75250 ±6 ms270 ±6 ms+20.5 ms (+8.2%)MAYBE
    p90260 ±7 ms287 ±14 ms+26.4 ms (+10.1%)NO

    20 test runs in comparison
    CommitTest Runs
    58b9da4
    • 2025-04-09_15:36:07.306830_Qqzs
    • 2025-04-09_15:36:07.306869_CaiJ
    • 2025-04-09_15:36:07.306879_KHBY
    • 2025-04-09_15:36:07.306888_uxgf
    • 2025-04-09_15:36:07.306895_vlYG
    • 2025-04-09_15:36:07.306901_lrMv
    • 2025-04-09_15:36:07.306908_Jrnt
    • 2025-04-09_15:36:07.306914_OqXO
    • 2025-04-09_15:36:07.306920_mPdN
    • 2025-04-09_15:36:07.306927_Lsdh
    2b6cb5b
    • 2025-04-09_16:06:07.568298_eqBw
    • 2025-04-09_16:06:07.568336_Xjll
    • 2025-04-09_16:06:07.568346_eNrM
    • 2025-04-09_16:06:07.568354_xyUn
    • 2025-04-09_16:06:07.568362_rlSf
    • 2025-04-09_16:06:07.568369_AWdl
    • 2025-04-09_16:06:07.568375_xIvI
    • 2025-04-09_16:06:07.568382_lqiN
    • 2025-04-09_16:06:07.568388_LSFO
    • 2025-04-09_16:06:07.568395_eNel

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

@tusharkhandelwal8 tusharkhandelwal8 marked this pull request as ready for review April 9, 2025 08:29
Copy link

@ashish-kothari ashish-kothari left a comment

Choose a reason for hiding this comment

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

LGTM, minor feedback on a code comment.

@VinayGuthal VinayGuthal self-requested a review April 9, 2025 15:55
@VinayGuthal VinayGuthal merged commit 5081e7c into main Apr 9, 2025
37 of 38 checks passed
@VinayGuthal VinayGuthal deleted the realtime-background-connections-fix branch April 9, 2025 16:22
@firebase firebase locked and limited conversation to collaborators May 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants