Skip to content

Fireperf: create a random delay before initiating a remote config fetch. #2968

Merged
leotianlizhan merged 8 commits into
masterfrom
fireperf-rc-fetch-delay
Sep 15, 2021
Merged

Fireperf: create a random delay before initiating a remote config fetch. #2968
leotianlizhan merged 8 commits into
masterfrom
fireperf-rc-fetch-delay

Conversation

@leotianlizhan

Copy link
Copy Markdown
Contributor

Rationale:
This is required to ensure that a remote notification does not trigger multiple config fetches from remote config at the same time. Internal bug reference: b/187985523

iOS version of the same change firebase/firebase-ios-sdk#8593

@google-oss-bot

google-oss-bot commented Sep 10, 2021

Copy link
Copy Markdown
Collaborator

Coverage Report

Affected SDKs

  • firebase-perf

    SDK overall coverage changed from 70.74% (b9f0ab6) to 70.79% (44dc407b) by +0.05%.

    Filename Base (b9f0ab6) Head (44dc407b) Diff
    RemoteConfigManager.java 92.24% 92.86% +0.62%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (44dc407b) is created by Prow via merging commits: b9f0ab6 b255861.

@google-oss-bot

google-oss-bot commented Sep 10, 2021

Copy link
Copy Markdown
Collaborator

Binary Size Report

Affected SDKs

  • firebase-perf

    Type Base (b9f0ab6) Head (44dc407b) Diff
    aar 299 kB 300 kB +463 B (+0.2%)
    apk (aggressive) 979 kB 980 kB +284 B (+0.0%)
    apk (release) 2.45 MB 2.45 MB +408 B (+0.0%)

Test Logs

Notes

Head commit (44dc407b) is created by Prow via merging commits: b9f0ab6 b255861.

@leotianlizhan

Copy link
Copy Markdown
Contributor Author

/retest

@visumickey visumickey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making this change @leotianlizhan. Glad to see the changes. I have few comments. PTAL as it permits.

@leotianlizhan

Copy link
Copy Markdown
Contributor Author

/retest

@visumickey visumickey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great! Thanks for making the changes Leo.

How are you planning to test this? It is important to do some manual testing to make sure we are able fetch the configs in less than 30 seconds and greater than 5 seconds. If you are able to ascertain that, please feel free to merge this change.

@leotianlizhan leotianlizhan force-pushed the fireperf-rc-fetch-delay branch from 2212970 to b255861 Compare September 15, 2021 15:20
@leotianlizhan

Copy link
Copy Markdown
Contributor Author

This is great! Thanks for making the changes Leo.

How are you planning to test this? It is important to do some manual testing to make sure we are able fetch the configs in less than 30 seconds and greater than 5 seconds. If you are able to ascertain that, please feel free to merge this change.

yes I've done some manual testing and it looks like it works

@leotianlizhan leotianlizhan merged commit 191700a into master Sep 15, 2021
@leotianlizhan leotianlizhan deleted the fireperf-rc-fetch-delay branch September 15, 2021 19:47
@firebase firebase locked and limited conversation to collaborators Oct 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes Override cla size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants