Skip to content

Commit

Permalink
Remove side effect of coupling op reentry and compression configs, re…
Browse files Browse the repository at this point in the history
…vert stress tests changes (microsoft#13619)

OP Reentry in the stress tests is still causing test incidents due to
the lifecycle of the container. This requires more refactoring of the
stress tests. Until then, these configs should be decoupled and the
reentry feature disabled in the stress tests in order to continue having
test coverage for compression.
  • Loading branch information
andre4i committed Jan 13, 2023
1 parent 9b091e2 commit 2c945c1
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 23 deletions.
5 changes: 1 addition & 4 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1049,10 +1049,7 @@ export class ContainerRuntime extends TypedEventEmitter<IContainerRuntimeEvents>
this.validateSummaryHeuristicConfiguration(this.summaryConfiguration);
}

this.enableOpReentryCheck = (runtimeOptions.enableOpReentryCheck === true
// If compression is enabled, we need to disallow op reentry as it is required that
// ops within the same batch have the same reference sequence number.
|| runtimeOptions.compressionOptions.minimumBatchSizeInBytes !== Number.POSITIVE_INFINITY)
this.enableOpReentryCheck = runtimeOptions.enableOpReentryCheck === true
// Allow for a break-glass config to override the options
&& this.mc.config.getBoolean("Fluid.ContainerRuntime.DisableOpReentryCheck") !== true;

Expand Down
21 changes: 3 additions & 18 deletions packages/test/test-service-load/src/loadTestDataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,8 @@ export class LoadTestDataStoreModel {
this.partnerId = (this.config.runId + halfClients) % this.config.testConfig.numClients;
const changed = (taskId) => {
if (taskId === this.taskId && this.taskStartTime !== 0) {
Promise.resolve().then(() => {
if (!this.runtime.disposed) {
this.dir.set(taskTimeKey, this.totalTaskTime);
this.taskStartTime = 0;
}
}).catch((error) => {
this.logger.sendErrorEvent({
eventName: "TaskManager_OnValueChanged",
}, error);
});
this.dir.set(taskTimeKey, this.totalTaskTime);
this.taskStartTime = 0;
}
};
this.taskManager.on("lost", changed);
Expand Down Expand Up @@ -264,14 +256,7 @@ export class LoadTestDataStoreModel {
: Math.trunc(value * blobsPerOp - this.blobCount);

if (newBlobs > 0) {
Promise.resolve().then(() => {
if (!this.runtime.disposed) {
this.blobUploads.push(...[...Array(newBlobs)].map(async () => this.writeBlob(this.blobCount++)));
}
}).catch((error) => this.logger.sendErrorEvent({
eventName: "WriteBlobFailed_OnCounterValueChanged",
count: this.blobCount,
}, error));
this.blobUploads.push(...[...Array(newBlobs)].map(async () => this.writeBlob(this.blobCount++)));
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/test/test-service-load/src/optionsMatrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function generateRuntimeOptions(
flushMode: [undefined],
compressionOptions: [{ minimumBatchSizeInBytes: 500, compressionAlgorithm: CompressionAlgorithms.lz4 }],
maxBatchSizeInBytes: [undefined],
enableOpReentryCheck: [true],
enableOpReentryCheck: [undefined],
chunkSizeInBytes: [undefined],
};

Expand Down

0 comments on commit 2c945c1

Please sign in to comment.