-
Notifications
You must be signed in to change notification settings - Fork 576
feat(container-loader): add readOnly option to loadFrozenContainerFromPendingState #27211
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
base: main
Are you sure you want to change the base?
Changes from all commits
3d57157
2991cf6
8f2774f
e771980
fb6a0fd
f4a202e
a08917f
3e767b8
7c31d95
ff37f19
411863f
d1fd645
7866ef5
cdcf203
849c0ed
ce7f4ed
e93eb26
ac44bd1
ebb5b29
8f15eef
86c509a
2035078
4ddd5c1
a3360ee
102c584
87fe785
dfcb644
78c9ef4
731aaea
e8dd9b9
7422e30
7312def
858990e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,11 @@ import { | |
| ReconnectMode, | ||
| } from "./contracts.js"; | ||
| import { DeltaQueue } from "./deltaQueue.js"; | ||
| import { FrozenDeltaStream, isFrozenDeltaStreamConnection } from "./frozenServices.js"; | ||
| import { | ||
| FrozenDeltaStream, | ||
| isFrozenDeltaStreamConnection, | ||
| isWritableFrozenDeltaStreamConnection, | ||
| } from "./frozenServices.js"; | ||
| import { SignalType } from "./protocol.js"; | ||
| import { isDeltaStreamConnectionForbiddenError } from "./utils.js"; | ||
|
|
||
|
|
@@ -582,21 +586,20 @@ export class ConnectionManager implements IConnectionManager { | |
| LogLevel.verbose, | ||
| ); | ||
| if (isDeltaStreamConnectionForbiddenError(origError)) { | ||
| connection = new FrozenDeltaStream(origError.storageOnlyReason, { | ||
| text: origError.message, | ||
| error: origError, | ||
| connection = new FrozenDeltaStream({ | ||
| storageOnlyReason: origError.storageOnlyReason, | ||
| readonlyConnectionReason: { text: origError.message, error: origError }, | ||
| }); | ||
| requestedMode = "read"; | ||
| break; | ||
| } else if ( | ||
| isFluidError(origError) && | ||
| origError.errorType === DriverErrorTypes.outOfStorageError | ||
| ) { | ||
| // If we get out of storage error from calling joinsession, then use the NoDeltaStream object so | ||
| // If we get out of storage error from calling joinsession, then use the FrozenDeltaStream object so | ||
| // that user can at least load the container. | ||
| connection = new FrozenDeltaStream(undefined, { | ||
| text: origError.message, | ||
| error: origError, | ||
| connection = new FrozenDeltaStream({ | ||
| readonlyConnectionReason: { text: origError.message, error: origError }, | ||
| }); | ||
| requestedMode = "read"; | ||
| break; | ||
|
|
@@ -1089,6 +1092,25 @@ export class ConnectionManager implements IConnectionManager { | |
|
|
||
| public sendMessages(messages: IDocumentMessage[]): void { | ||
| assert(this.connected, 0x2b4 /* "not connected on sending ops!" */); | ||
| // WritableFrozenDeltaStream short-circuit: writable-frozen containers | ||
|
anthony-murphy marked this conversation as resolved.
|
||
| // (`loadFrozenContainerFromPendingState({ readOnly: false })`) attach a | ||
| // WritableFrozenDeltaStream as the live connection. Its `mode` is "read" (advertising | ||
| // "write" would imply quorum membership we cannot honor), so a runtime submit | ||
| // would otherwise fall into the read-mode reconnect branch below. That branch | ||
| // schedules `reconnect("write")`, which under `ReconnectMode.Never` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there are other cases in the runtime that trigger reconnect that would cause trouble?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Walked through the runtime-reconnect triggers — they all funnel into
So the only path that would still trigger a runtime-driven write reconnect is a stray submit hitting the connection's Are you thinking of a specific scenario I haven't covered? Happy to add a test if there's a path I'm missing. |
||
| // (`allowReconnect: false`) calls `closeHandler` and closes the container — the | ||
| // opposite of what writable-frozen wants. Drop the messages here: the runtime's | ||
| // outbox keeps them in `pendingStateManager` so `getPendingLocalState()` can | ||
| // capture them, which is the entire point of the writable-frozen flow. | ||
| // | ||
| // Match only the writable variant (a sibling class, not a subclass) so the read-only | ||
| // `FrozenDeltaStream` retains its `submit` 403-nack tripwire — a stray submit on a | ||
| // storage-only frozen connection signals an upstream invariant break and should | ||
| // remain observable. The read-only variant shouldn't reach here in normal flow anyway | ||
| // (its `storageOnly` policy keeps the runtime from submitting). | ||
| if (isWritableFrozenDeltaStreamConnection(this.connection)) { | ||
| return; | ||
| } | ||
| // If connection is "read" or implicit "read" (got leave op for "write" connection), | ||
| // then op can't make it through - we will get a nack if op is sent. | ||
| // We can short-circuit this process. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.