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

Revert: Reset joinSession cache only on connect_document_error #20784

Conversation

pragya91
Copy link
Contributor

@pragya91 pragya91 commented Apr 22, 2024

Reverts #19974 PR which was applied as a performance improvement. In the original PR, we checked for the kind of error received on the socket to decide whether to clear joinSession cache or not and reuse the existing session whenever possible. However a bug was identified where one section of the code was not update to stamp the errorFrom property which is used to perform such check which resulted in the cache never clearing for and the code getting stuck in a loop to continue connection retries.

Impact of revert: There is no compatibility-issue/regression that this revert would introduce, as it was only a perf improvement.

Follow up: Captured in AB#7833

@github-actions github-actions bot added area: driver Driver related issues area: odsp-driver base: main PRs targeted against main branch labels Apr 22, 2024
@pragya91 pragya91 requested review from agarwal-navin, jatgarg and a team April 22, 2024 21:24
@pragya91 pragya91 marked this pull request as ready for review April 22, 2024 21:25
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: -66 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 452.8 KB 452.8 KB No change
azureClient.js 549.21 KB 549.2 KB -12 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 256.01 KB 256.01 KB No change
fluidFramework.js 338.57 KB 338.57 KB No change
loader.js 133.01 KB 133.01 KB No change
map.js 41.42 KB 41.42 KB No change
matrix.js 143.43 KB 143.43 KB No change
odspClient.js 517.56 KB 517.56 KB No change
odspDriver.js 97.52 KB 97.52 KB No change
odspPrefetchSnapshot.js 42.39 KB 42.39 KB No change
sharedString.js 161.24 KB 161.24 KB No change
sharedTree.js 338.57 KB 338.57 KB No change
Total Size 3.15 MB 3.15 MB -66 Bytes

Baseline commit: 8d501fc

Generated by 🚫 dangerJS against 7955d19

@pragya91 pragya91 merged commit 3342aff into microsoft:main Apr 22, 2024
34 checks passed
pragya91 added a commit to pragya91/FluidFramework-1 that referenced this pull request Apr 22, 2024
…soft#20784)

Reverts microsoft#19974 PR which
was applied as a performance improvement. In the original PR, we checked
for the kind of error received on the socket to decide whether to clear
joinSession cache or not and reuse the existing session whenever
possible. However a bug was identified where one section of the code was
not update to stamp the errorFrom property which is used to perform such
check which resulted in the cache never clearing for and the code
getting stuck in a loop to continue connection retries.

Impact of revert: There is no compatibility-issue/regression that this
revert would introduce, as it was only a perf improvement.

Follow up: Captured in
[AB#7833](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/7833)
pragya91 added a commit that referenced this pull request Apr 22, 2024
… (#20789)

Reverts #19974 PR which
was applied as a performance improvement. In the original PR, we checked
for the kind of error received on the socket to decide whether to clear
joinSession cache or not and reuse the existing session whenever
possible. However a bug was identified where one section of the code was
not update to stamp the errorFrom property which is used to perform such
check which resulted in the cache never clearing for and the code
getting stuck in a loop to continue connection retries.

Impact of revert: There is no compatibility-issue/regression that this
revert would introduce, as it was only a perf improvement.

Follow up: Captured in

[AB#7833](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/7833)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: driver Driver related issues area: odsp-driver base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants