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

Remove batch manager with feature gate #7178

Merged
merged 56 commits into from
Sep 7, 2021
Merged
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
fd24c4c
Rename flushmodes
andre4i Aug 6, 2021
c93177f
Remove from examples
andre4i Aug 6, 2021
e43bfda
Fix formatting
andre4i Aug 6, 2021
fb3692e
Rename rest of FlushMode.manual
andre4i Aug 6, 2021
0005e8f
update api md, remove unused imports, remove redudant settings
andre4i Aug 6, 2021
06aecd8
Adjust local server tests
andre4i Aug 6, 2021
b2d14a4
Attempt to fix end to end tests
andre4i Aug 6, 2021
e30aef0
Retrofit all tests
andre4i Aug 9, 2021
7a36105
Merge branch 'main' into default-flushmode-manual
andre4i Aug 9, 2021
6bb2ccf
Reorder properties
andre4i Aug 9, 2021
10be3fd
Add notes to BREAKING.md
andre4i Aug 9, 2021
f546255
Fix anchor link in BREAKING.md
andre4i Aug 9, 2021
9a21e38
Add framework name to the BREAKING.md note
andre4i Aug 9, 2021
fdf4911
Some minor refactoring
andre4i Aug 9, 2021
645285a
Minor refactoring
andre4i Aug 9, 2021
2253580
Remove uneless setFlushMode
andre4i Aug 9, 2021
466c1fb
Merge branch 'main' into default-flushmode-manual
andre4i Aug 9, 2021
8ee49f0
Merge branch 'main' into default-flushmode-manual
andre4i Aug 10, 2021
04290f3
Remove BatchManager
andre4i Aug 10, 2021
28101a6
Fix API doc, fix localdocumentdeltaconnection
andre4i Aug 10, 2021
2d2d494
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 10, 2021
f754c38
Merge branch 'main' of github.com:andre4i/FluidFramework into remove-…
andre4i Aug 10, 2021
b0ea4bd
Merge branch 'main' into default-flushmode-manual
andre4i Aug 10, 2021
d0eef76
Merge branch 'main' into default-flushmode-manual
andre4i Aug 13, 2021
63fa1c6
Merge branch 'main' into default-flushmode-manual
andre4i Aug 16, 2021
7541af4
Add note about events
andre4i Aug 16, 2021
a75aaac
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 16, 2021
47fc3cc
Fix expected vs actual assert, minor refactoring, fix a couple of tests
andre4i Aug 16, 2021
e462128
Trying to fix some tests
andre4i Aug 17, 2021
035f455
Fix sharedinterval tests, remove yield
andre4i Aug 17, 2021
f4180a4
Some PR feedback
andre4i Aug 18, 2021
f135b61
Merge branch 'main' into default-flushmode-manual
andre4i Aug 19, 2021
2461cbe
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 19, 2021
3089e9c
Soften up the breaking.md text
andre4i Aug 19, 2021
c1a7062
Merge branch 'main' into default-flushmode-manual
andre4i Aug 19, 2021
b2f866d
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 19, 2021
00e86e6
Feature gate for batch manager
andre4i Aug 19, 2021
b3d19aa
Featuregate batch manager disablement
andre4i Aug 19, 2021
a68e7d9
Feature gate for default flush mode
andre4i Aug 19, 2021
22fc385
Remove redundant true comparison
andre4i Aug 19, 2021
e1ce870
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 19, 2021
8802ecd
Extract magic string
andre4i Aug 19, 2021
2d3ec02
Merge branch 'main' into default-flushmode-manual
andre4i Aug 20, 2021
0ff976d
Merge branch 'main' into default-flushmode-manual
andre4i Aug 20, 2021
97b8278
Merge branch 'main' into default-flushmode-manual
andre4i Aug 20, 2021
f84d43d
Update breaking.md
andre4i Aug 20, 2021
0546fb6
Merge branch 'main' into default-flushmode-manual
andre4i Aug 24, 2021
6f10b58
PR feedback
andre4i Aug 24, 2021
3664f7d
Merge branch 'main' into default-flushmode-manual
andre4i Aug 24, 2021
437307a
Merge branch 'default-flushmode-manual' into remove-batch-manager-2
andre4i Aug 24, 2021
93c2d52
Merge branch 'main' into remove-batch-manager-2
andre4i Aug 24, 2021
d8bf3f2
make method static as it's stateles
andre4i Aug 24, 2021
992559f
Remove space
andre4i Aug 24, 2021
c09fdfe
Merge branch 'main' into remove-batch-manager-2
andre4i Sep 2, 2021
36f70f9
Merge branch 'main' into remove-batch-manager-2
andre4i Sep 3, 2021
129ff47
Merge branch 'main' into remove-batch-manager-2
andre4i Sep 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api-report/driver-base.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export class DocumentDeltaConnection extends TypedEventEmitter<IDocumentDeltaCon
// (undocumented)
protected earlySignalHandler: (msg: ISignalMessage) => void;
// (undocumented)
protected emitMessages(type: string, messages: IDocumentMessage[][]): void;
// (undocumented)
static readonly eventsAlwaysForwarded: string[];
// (undocumented)
static readonly eventsToForward: string[];
Expand All @@ -60,6 +62,8 @@ export class DocumentDeltaConnection extends TypedEventEmitter<IDocumentDeltaCon
get initialMessages(): ISequencedDocumentMessage[];
get initialSignals(): ISignalMessage[];
// (undocumented)
protected readonly isBatchManagerDisabled: boolean;
// (undocumented)
protected readonly logger: ITelemetryLogger;
get maxMessageSize(): number;
get mode(): ConnectionMode;
Expand All @@ -72,6 +76,8 @@ export class DocumentDeltaConnection extends TypedEventEmitter<IDocumentDeltaCon
protected readonly socket: SocketIOClient.Socket;
submit(messages: IDocumentMessage[]): void;
// (undocumented)
protected submitCore(type: string, messages: IDocumentMessage[]): void;
// (undocumented)
protected readonly submitManager: BatchManager<IDocumentMessage[]>;
submitSignal(message: IDocumentMessage): void;
get version(): string;
Expand Down
47 changes: 36 additions & 11 deletions packages/drivers/driver-base/src/documentDeltaConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { assert , BatchManager, TypedEventEmitter } from "@fluidframework/common-utils";
import { assert, BatchManager, TypedEventEmitter } from "@fluidframework/common-utils";
import {
IDocumentDeltaConnection,
IDocumentDeltaConnectionEvents,
Expand All @@ -25,6 +25,9 @@ import {
import { IDisposable, ITelemetryLogger } from "@fluidframework/common-definitions";
import { ChildLogger } from "@fluidframework/telemetry-utils";

// Local storage key to disable the BatchManager
const batchManagerDisabledKey = "FluidDisableBatchManager";

/**
* Represents a connection to a stream of delta updates
*/
Expand Down Expand Up @@ -80,6 +83,7 @@ export class DocumentDeltaConnection
*/
protected _disposed: boolean = false;
protected readonly logger: ITelemetryLogger;
protected readonly isBatchManagerDisabled: boolean = false;

public get details(): IConnected {
if (!this._details) {
Expand All @@ -102,14 +106,7 @@ export class DocumentDeltaConnection
this.logger = ChildLogger.create(logger, "DeltaConnection");

this.submitManager = new BatchManager<IDocumentMessage[]>(
(submitType, work) => {
// Although the implementation here disconnects the socket and does not reuse it, other subclasses
// (e.g. OdspDocumentDeltaConnection) may reuse the socket. In these cases, we need to avoid emitting
// on the still-live socket.
if (!this.disposed) {
this.socket.emit(submitType, this.clientId, work);
}
});
(submitType, work) => this.emitMessages(submitType, work));

this.on("newListener", (event, listener) => {
assert(!this.disposed, 0x20a /* "register for event on disposed object" */);
Expand Down Expand Up @@ -138,6 +135,8 @@ export class DocumentDeltaConnection
});
}
});

this.isBatchManagerDisabled = DocumentDeltaConnection.disabledBatchManagerFeatureGate;
}

/**
Expand Down Expand Up @@ -259,14 +258,40 @@ export class DocumentDeltaConnection
return this.details.initialClients;
}

protected emitMessages(type: string, messages: IDocumentMessage[][]) {
// Although the implementation here disconnects the socket and does not reuse it, other subclasses
// (e.g. OdspDocumentDeltaConnection) may reuse the socket. In these cases, we need to avoid emitting
// on the still-live socket.
if (!this.disposed) {
this.socket.emit(type, this.clientId, messages);
}
}

private static get disabledBatchManagerFeatureGate() {
try {
return localStorage !== undefined
&& typeof localStorage === "object"
&& localStorage.getItem(batchManagerDisabledKey) === "1";
} catch (e) { }
return false;
}

protected submitCore(type: string, messages: IDocumentMessage[]) {
if (this.isBatchManagerDisabled) {
this.emitMessages(type, [messages]);
} else {
this.submitManager.add(type, messages);
}
}

/**
* Submits a new delta operation to the server
*
* @param message - delta operation to submit
*/
public submit(messages: IDocumentMessage[]): void {
this.checkNotClosed();
this.submitManager.add("submitOp", messages);
this.submitCore("submitOp", messages);
}

/**
Expand All @@ -276,7 +301,7 @@ export class DocumentDeltaConnection
*/
public submitSignal(message: IDocumentMessage): void {
this.checkNotClosed();
this.submitManager.add("submitSignal", [message]);
this.submitCore("submitSignal", [message]);
}

/**
Expand Down
15 changes: 11 additions & 4 deletions packages/drivers/local-driver/src/localDocumentDeltaConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,31 @@ export class LocalDocumentDeltaConnection extends DocumentDeltaConnection {
super(socket, documentId, new TelemetryNullLogger());
}

protected submitCore(type: string, messages: IDocumentMessage[]) {
if (this.isBatchManagerDisabled) {
this.emitMessages(type, [messages]);
} else {
this.submitManager.add(type, messages);
this.submitManager.drain();
}
}

/**
* Submits a new delta operation to the server
*/
public submit(messages: IDocumentMessage[]): void {
// We use a promise resolve to force a turn break given message processing is sync
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Promise.resolve().then(() => {
this.submitManager.add("submitOp", messages);
this.submitManager.drain();
this.submitCore("submitOp", messages);
});
}

/**
* Submits a new signal to the server
*/
public submitSignal(message: any): void {
this.submitManager.add("submitSignal", message);
this.submitManager.drain();
this.submitCore("submitSignal", [message]);
}

/**
Expand Down
Loading