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

Fix op compression when using an older loader #14337

Merged
merged 8 commits into from
Feb 27, 2023

Conversation

andre4i
Copy link
Contributor

@andre4i andre4i commented Feb 27, 2023

Description

AB#3538

For loaders older than client_v2.0.0-internal.1.2.0 (exclusive), compression gracefully does not work (it does happen, but the messages are eventually sent uncompressed over the wire)

However, for loaders between client_v2.0.0-internal.1.2.0 (inclusive) and client_v2.0.0-internal.2.2.0 (exclusive), the runtime will provide the loader with the compression field as such:

this.params.containerContext.submitBatchFn(
                batch.content.map((message) => ({
                    contents: message.contents,
                    metadata: message.metadata,
                    compression: message.compression, // <-- compression marker
                    referenceSequenceNumber: message.referenceSequenceNumber,
                })),
                batch.referenceSequenceNumber,
            );

but the delta manager, on the loader side will ignore that field. Therefore, the message is sent over the wire without any compression markers. This means that no client is able to process this compressed message as it has no compression marker, and the container will close with assert 0x3ce (Runtime message of unknown type).

To work around this, the runtime must figure out if the message is compressed based on its 'shape'

This change also replaces the assert with an explicit data processing error.

@andre4i andre4i requested review from a team as code owners February 27, 2023 16:36
@github-actions github-actions bot added area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Feb 27, 2023
@github-actions github-actions bot removed the area: tests Tests to add, test infrastructure improvements, etc label Feb 27, 2023
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Feb 27, 2023

@fluid-example/bundle-size-tests: +1.77 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 432.49 KB 433.37 KB +907 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 227.59 KB 228.48 KB +909 Bytes
loader.js 152.93 KB 152.93 KB No change
map.js 43.78 KB 43.78 KB No change
matrix.js 135.95 KB 135.95 KB No change
odspDriver.js 91.79 KB 91.79 KB No change
odspPrefetchSnapshot.js 43.51 KB 43.51 KB No change
sharedString.js 156.5 KB 156.5 KB No change
Total Size 1.36 MB 1.36 MB +1.77 KB

Baseline commit: 1df94c8

Generated by 🚫 dangerJS against 7b3b79b

@github-actions github-actions bot added the area: tests Tests to add, test infrastructure improvements, etc label Feb 27, 2023
@andre4i andre4i merged commit cb7defa into microsoft:main Feb 27, 2023
andre4i added a commit that referenced this pull request Feb 27, 2023
## Description

Porting #14337 to the
release branch


# NOTE TO REVIEWERS

**As
[release/v2int/3.0](https://github.com/microsoft/FluidFramework/tree/release/v2int/3.0)
was cut before we moved to tabs, this PR was not done using
`cherry-pick`. It was done manually on a fresh branch off the review
branch. Please spot check this and be extra careful about the diff
between this and
#14337
andre4i added a commit to andre4i/FluidFramework that referenced this pull request Feb 28, 2023
## Description

ADO:3538

For loaders older than client_v2.0.0-internal.1.2.0 (exclusive),
compression gracefully does not work (it does happen, but the messages
are eventually sent uncompressed over the wire)

However, for loaders between client_v2.0.0-internal.1.2.0 (inclusive)
and client_v2.0.0-internal.2.2.0 (exclusive), the runtime will provide
the loader with the compression field but the delta manager, on the loader side will ignore that field.
Therefore, the message is sent over the wire without any compression
markers. This means that no client is able to process this compressed
message as it has no compression marker, and the container will close
with assert `0x3ce` (`Runtime message of unknown type`).

**To work around this, the runtime must figure out if the message is
compressed based on its 'shape'**

This change also replaces the assert with an explicit data processing
error.
andre4i added a commit that referenced this pull request Feb 28, 2023
## Description
Preemptively porting
#14337 to the 3.2
release branch.

As opposed to #14345,
this was cherry-picked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants