Skip to content

Commit

Permalink
Add diagnostics to ConsensusQueue in handle validation tests (microso…
Browse files Browse the repository at this point in the history
…ft#22531)

## Description

Modifies `ConsensusQueue`'s generic "store a piece of data"
implementation in `handleValidation.ts` to give more information on a
couple theoretical error methods. This test timed out on a recent
tinylicious run where based on telemetry, it seems like the test got
past the point of loading the 2nd container. Thus it was likely a
deadlock awaiting this promise. I've been unable to reproduce this
behavior locally. The goal here is to get information about the failure
mode the next time this race condition happens.

---------

Co-authored-by: Abram Sanderson <[email protected]>
  • Loading branch information
Abe27342 and Abram Sanderson authored Sep 16, 2024
1 parent c58af5c commit 21288ba
Showing 1 changed file with 20 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
createAndAttachContainer,
ITestFluidObject,
type ITestObjectProvider,
timeoutAwait,
} from "@fluidframework/test-utils/internal";
import {
ITree,
Expand Down Expand Up @@ -376,14 +377,25 @@ describeCompat("handle validation", "NoCompat", (getTestObjectProvider, apis) =>
await queue.add(handle);
},
async readHandle(): Promise<unknown> {
const value = await new Promise((resolve, reject) => {
queue
.acquire(async (result: FluidObject) => {
resolve(result);
return ConsensusResult.Release;
})
.catch((error) => reject(error));
});
const value = await timeoutAwait(
new Promise((resolve, reject) => {
queue
.acquire(async (result: FluidObject) => {
resolve(result);
return ConsensusResult.Release;
})
.then((wasNonempty) => {
if (!wasNonempty) {
// This could happen if a test never calls `storeHandle` before attempting to read it.
// Other modes of failure are possible (e.g. correctness issues in ConsensusQueue causing it to have the wrong data).
// Resolving the promise with `undefined` here could be another reasonable option, but this gives slightly more information.
reject(new Error("No values found in consensus queue."));
}
})
.catch((error) => reject(error));
}),
{ errorMsg: "Timeout waiting for acquiring value from consensus queue." },
);
return value;
},
handle: queue.handle,
Expand Down

0 comments on commit 21288ba

Please sign in to comment.