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

Enable the Summary Cache #12036

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Conversation

tianzhu007
Copy link
Contributor

The Powerapp would have a latency issue on FRS, since the client would open the new session from the Storage every time, instead of using the cache. We would enable the cache, in this case, to speed up the process.

@tianzhu007 tianzhu007 requested a review from a team as a code owner September 19, 2022 20:05
@github-actions github-actions bot added area: server Server related issues (routerlicious) base: main PRs targeted against main branch labels Sep 19, 2022
@tianzhu007 tianzhu007 merged commit f4a7c43 into microsoft:main Sep 19, 2022
@github-actions
Copy link
Contributor

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

tylerbutler pushed a commit to tylerbutler/FluidFramework that referenced this pull request Sep 26, 2022
The Powerapp would have a latency issue on FRS, since the client would
open the new session from the Storage every time, instead of using the
cache. We would enable the cache, in this case, to speed up the process.
@tianzhu007 tianzhu007 changed the title Remove the Summary Cache Enable the Summary Cache Sep 27, 2022
znewton added a commit that referenced this pull request Mar 23, 2023
## Description

Historian caches latest summary, but it doesn't check if a requested
summary version is already cached as latest.

A bug in the R11s-driver was recently fixed in #14400 where the driver
will request latest summary, then also request it again by its ID. This
is fixed in the driver now, but will take some time to saturate clients
using r11s-driver. We can lessen the impact of this bug on the server by
causing that 2nd read to be a Cache Hit instead of a 100% miss.

## Breaking Changes

None, but I removed a commented out section of code from #12036 and
replaced it with a reference to an issue I opened to track it instead
(#14623).

## Reviewer Guidance

The goal here is to check if the requested summary sha is the same as
the cached latest summary sha. However, we do not want to read the
entire summary from cache into memory just to read its sha, in case it
is not a cache hit. Instead, I added an additional cache key which will
store specifically the cached latest summary's sha, which will improve
efficiency of the sha equality check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: server Server related issues (routerlicious) base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants