Skip to content

Commit

Permalink
Empty compression messages (microsoft#14261)
Browse files Browse the repository at this point in the history
## Description

~We've identified an issue with compression, that the trailing empty ops
added to the batch to preserve the batch shape and reserve sequence
numbers are not really empty, as the content is still present in the
`deserializedContent` property. This leads to compression creating a
large payload which does not end up getting chunked.~

This is not actually an issue. When compression is not supported by the
loader, the outbox will send the `deserializedContent` property. When it
is, a message will be crafted to only contain contents, metadata,
compression and referenceSequenceNumber. So if compression is enabled
and supported, the message size will actually be small as the
`deserializedContent` value is discarded. This is still a good change,
it will make memory consumption smaller when compression is enabled and
supported.
  • Loading branch information
andre4i committed Mar 1, 2023
1 parent f258b44 commit 30a2205
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,16 @@ export class OpCompressor {
});

for (const message of batch.content.slice(1)) {
messages.push({ ...message, contents: undefined });
// Add an empty placeholder message to reserve the sequence numbers
messages.push({
deserializedContent: {
contents: undefined,
type: message.deserializedContent.type,
},
localOpMetadata: undefined,
metadata: undefined,
referenceSequenceNumber: message.referenceSequenceNumber,
});
}

const compressedBatch: IBatch = {
Expand Down
10 changes: 5 additions & 5 deletions packages/runtime/container-runtime/src/opLifecycle/outbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,12 @@ export class Outbox {
if (this.params.containerContext.submitBatchFn === undefined) {
// Legacy path - supporting old loader versions. Can be removed only when LTS moves above
// version that has support for batches (submitBatchFn)
for (const message of batch.content) {
// Legacy path doesn't support compressed payloads and will submit uncompressed payload anyways
if (message.metadata?.compressed) {
delete message.metadata.compressed;
}
assert(
batch.content[0].compression === undefined,
"Compression should not have happened if the loader does not support it",
);

for (const message of batch.content) {
this.params.containerContext.submitFn(
MessageType.Operation,
message.deserializedContent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ describe("OpCompressor", () => {
if (compressedBatch.content.length > 1) {
assert.strictEqual(compressedBatch.content[1].contents, undefined);
assert.strictEqual(compressedBatch.content[1].compression, undefined);
assert.strictEqual(compressedBatch.content[1].metadata, undefined);
assert.strictEqual(compressedBatch.content[1].localOpMetadata, undefined);
assert.strictEqual(
compressedBatch.content[1].deserializedContent.contents,
undefined,
);
}
});
}));
Expand Down

0 comments on commit 30a2205

Please sign in to comment.