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

Copy of the container with compressed binary summaries is failing. #17439

Open
milanro opened this issue Sep 22, 2023 · 6 comments
Open

Copy of the container with compressed binary summaries is failing. #17439

milanro opened this issue Sep 22, 2023 · 6 comments
Labels
ado Issue is tracked in the internal Azure DevOps backlog bug Something isn't working

Comments

@milanro
Copy link
Contributor

milanro commented Sep 22, 2023

When using summary compression and generating compressed binary summary blobs and executing the AzureClient.copyContainer method, it is failing.

At first it fails in the assertion

 assert (assert.js:17)
getSnapshotTreeFromSerializedContainer (utils.js:110)
rehydrateDetachedFromSnapshot (container.js:1122)
_fluidframework_telemetry_utils__WEBPACK_IMPORTED_MODULE_6__.PerformanceEvent.timedExecAsync.start (container.js:386)
timedExecAsync (logger.js:432)
rehydrateDetachedFromSnapshot (container.js:384)
rehydrateDetachedContainerFromSnapshot (loader.js:145)
copyContainer (AzureClient.js:90)

where the assertion is called at packages/loader/container-loader/src/utils.ts. The reason for the wrong assertion is in the function packages/loader/driver-utils/src/summaryForCreateNew.ts#isCombinedAppAndProtocolSummary, which expects exactly 2 keys(.app and .protocol)in the summary tree where we receive 4 keys : (.app, .protocol, .logTail, .serviceProtocol).

If this failing place is fixed and 2 or more keys are allowed, it fails on the next place. It fails with the following stack

Uint8ArrayToArrayBuffer (bufferShared.js:15)
convertSummaryToSnapshotWithEmbeddedBlobContents (utils.js:78)
convertProtocolAndAppSummaryToSnapshotTree (utils.js:104)
getSnapshotTreeFromSerializedContainer (utils.js:113)
rehydrateDetachedFromSnapshot (container.js:1122)

The failing function is as follows 👍

export function Uint8ArrayToArrayBuffer(array) {
    if (array.byteOffset === 0 && array.byteLength === array.buffer.byteLength) {
        return array.buffer;
    }
    return array.buffer.slice(array.byteOffset, array.byteOffset + array.byteLength);
}

The problem is at the array.buffer.slice where the array.buffer is undefined.

The reason for this is in passing the summary parameter. The summary is converted to json at

AzureClient.copyContainer#143
const container = await loader.rehydrateDetachedContainerFromSnapshot(JSON.stringify(tree));

and then converted back at packages/loader/container-loader/src/container.ts#rehydrateDetachedFromSnapshot
This leads to incorrect representation of our binary blob which is represented for example as follows :

{
"0": 123,
"1": 34,
"2": 101,
"3": 108,
"4": 101,
"5": 99,
"6": 116,
...
}

Test example can be initialized and executed as follows :

  • configure environment variables as follows :
FLUID_MODE=frs
SECRET_FLUID_RELAY=<FRS url, for example https://eu.fluidrelay.azure.com>
SECRET_FLUID_TOKEN=<token>
SECRET_FLUID_TENANT=<tenant>
  • git clone --single-branch --branch CompressedSummaryCopy git@github.com:milanro/FluidFramework.git FluidBinCopy
  • cd FluidBinCopy
  • pnpm i
  • npm run build:fast
  • cd examples/data-objects/miso_tinybinsummary/
  • npm start
  • in browser : http://localhost:8080/
  • Wait for summarization (a few minutes as there is no log)
  • press CP in the web page to start container copy

@DLehenbauer @dstanesc @vladsud

@milanro milanro added the bug Something isn't working label Sep 22, 2023
@milanro
Copy link
Contributor Author

milanro commented Sep 25, 2023

@ssimic2 please, check

@DLehenbauer
Copy link
Contributor

DLehenbauer commented Oct 18, 2023

@milanro - This turned out to be unrelated to the other issue we were debugging where copyContainer was implicated.

As of Monday, it's been given its own tracking work item (AB#5935) and assigned to a dev for investigation.

@milanro
Copy link
Contributor Author

milanro commented Oct 18, 2023

@milanro - This turned out to be unrelated to the other issue we were debugging where copyContainer was implicated.

As of Monday, it's been given its own tracking work item (AB#5935) and assigned to a dev for investigation.

@DLehenbauer Unfortunatelly I have no access to this work item.

@DLehenbauer
Copy link
Contributor

Yeah, sorry... that link was for my own reference to help me track progress.

@RishhiB
Copy link
Contributor

RishhiB commented Oct 19, 2023

Hi @milanro we are taking a look at this. I am able to reproduce this bug using your webapp.

Fluid packages that contain this issue;
"@fluidframework/azure-client": "2.0.0-internal.6.4.0",
"@fluidframework/cell": "2.0.0-internal.6.4.0",
"@fluidframework/test-runtime-utils": "2.0.0-internal.6.4.0",
"@fluid-tools/webpack-fluid-loader": "2.0.0-internal.6.4.0",
"@fluidframework/build-common": "^2.0.0",
"@fluidframework/eslint-config-fluid": "^2.1.0",

@RishhiB
Copy link
Contributor

RishhiB commented Oct 25, 2023

Hey @milanro. Just a heads up we have taken a look at this issue and produced some findings. We are currently backlogging this issue so we can plan for it properly. We will give an update when it is scheduled to be worked on again. Thank you!

@kashms kashms added the ado Issue is tracked in the internal Azure DevOps backlog label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ado Issue is tracked in the internal Azure DevOps backlog bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants