Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary #12313

NicholasCouri · 2022-10-06T18:51:17Z

Description

The refreshLatestSummaryAck currently retrieves a version of a summary based on the summaryAckHandle received. The proposal is to ignore that handle and always retrieve the latest version (explicitly ignoring the Ack Handle).
In case the delta manager is behind the latest summary's ref seq number we would wait for it to catch up and proceed with the same existing logic.

As noted during the meeting, I'm still in the process of validating the change and the end-to-end test was an initial attempt to validate it but I'd love to get feedback in case there are scenarios that I might be breaking or something I'm missing

# Conflicts: # packages/runtime/container-runtime/src/containerRuntime.ts

msfluid-bot · 2022-10-06T19:16:11Z

⯅ @fluid-example/bundle-size-tests: +672 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	403.94 KB	404.27 KB	⯅ +336 Bytes
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	201.1 KB	201.43 KB	⯅ +336 Bytes
loader.js	153.8 KB	153.8 KB	■ No change
map.js	47.39 KB	47.39 KB	■ No change
matrix.js	138.69 KB	138.69 KB	■ No change
odspDriver.js	118.76 KB	118.76 KB	■ No change
odspPrefetchSnapshot.js	40.54 KB	40.54 KB	■ No change
sharedString.js	158.29 KB	158.29 KB	■ No change
Total Size	1.3 MB	1.3 MB	⯅ +672 Bytes

Baseline commit: 4fadd10

Generated by 🚫 dangerJS against 5f43213

packages/runtime/container-runtime/src/containerRuntime.ts

vladsud · 2022-10-07T00:06:08Z

I do not think your code changes that, but do we test and does it work correctly IF summarizer has pending summary, but we get an ack about some other summary first, followed by nack for our summary? Or maybe even no nack, if PUSH crashed somewhere in between?

vladsud · 2022-10-07T00:08:52Z

Can you please share a bit more info when we retrieve summary by handle? It feels like with summarizer always starting to wait for summary ops/acks after fully being connected, it should happen ONLY if we have 2 summarizers in the session. While we know this is what actually happens (effectively) with FRS service summaries, where can it happen for SPO? It clearly happens as I hear feedback about it :) I want to make sure we walk that scenarios explicitly - mentally through the code, but also replicate in end-to-end tests, and ensure that it continues to work correctly

…izer nodes.

NicholasCouri · 2022-10-07T17:56:01Z

I have added telemetry fluid:telemetry:SummarizerNode:PendingSummaryNotFound to find out when/if it happens and it only happened 6 times (FRS) out of 15k hits in the test db. I'll investigate those. The majority of the time it is a summary ack and we have no pending summaries. Here is a query I was looking at:

union database("Office Fluid Test").office_fluid_ff*
| where Data_hostName == "@fluid-internal/test-service-load"
| where Data_eventName == "fluid:telemetry:SummarizerNode:PendingSummaryNotFound"
| extend propJson = parse_json(Data_details)
| extend summaryRefSeq = propJson.summaryRefSeq
| extend pendingSize = propJson.pendingSize
| summarize Total=count(), PendingSizeNotZero=countif(pendingSize > 0),
PendingSizeNotZeroAndREfSeq0=countif(pendingSize > 0 and Data_referenceSequenceNumber == 0),
RefSeqNumber0=countif(Data_referenceSequenceNumber == 0),
NoUpdate=countif(Data_referenceSequenceNumber == summaryRefSeq),
DidFetchSnapshot=countif(Data_referenceSequenceNumber < summaryRefSeq) by Data_driverType, Data_driverEndpointName

In reply to: 1270858639

NicholasCouri · 2022-10-07T17:59:51Z

I suppose we do try to get summaries by handle whenever we get the latest snapshot from cache and while trying to catch up identify the summary acks among the ops and handlesummaryAcks is called on to refresh the state. The query I shared previously shows how often it happens (pendingSize == 0) on our Office Fluid Test db.

In reply to: 1270860197

packages/runtime/container-runtime/src/containerRuntime.ts

vladsud · 2022-10-14T16:15:41Z

I suppose we do try to get summaries by handle whenever we get the latest snapshot from cache and while trying to catch up identify the summary acks among the ops and handlesummaryAcks is called on to refresh the state. The query I shared previously shows how often it happens (pendingSize == 0) on our Office Fluid Test db.

I'd love to confirm that. We should not used cached snapshots for summarizer today, but there still might be a race condition while loading those results in that flow.
Conforming for sure is important because it helps us answer two key questions:

With these changes, are we ready to enable loading summarizer from cache? And if so, is it efficient? I.e. I'd think that sequence results in a bunch of calls to getLatest in a row very tight in time, and maybe we should find a way not to do some of these calls (summarizer loading from cache will trigger refresh of cache. When summarizer detects it's not current, it will ask for refresh, which could return results from cache (cache is almost new), and not do another network call).
Understanding if this is strictly boot problem, and if so, considering making this code run only on boot (not having a loop) and push all other cases on "regular" recovery path that involves reboot of summarizer.

packages/test/test-end-to-end-tests/src/test/SummarizeFromLatest.spec.ts

NicholasCouri · 2022-10-14T17:10:26Z

Can you please share a bit more info when we retrieve summary by handle? It feels like with summarizer always starting to wait for summary ops/acks after fully being connected, it should happen ONLY if we have 2 summarizers in the session. While we know this is what actually happens (effectively) with FRS service summaries, where can it happen for SPO? It clearly happens as I hear feedback about it :) I want to make sure we walk that scenarios explicitly - mentally through the code, but also replicate in end-to-end tests, and ensure that it continues to work correctly

I've added some information to Task 444 - it seems the first time the summarizer is launched, after catching up, the handleSummaryAcks does receive the last ack from the summaryCollection before its processing has started. Its local reference number is still 0 and we query the snapshot.

agarwal-navin · 2022-10-26T21:29:34Z

packages/runtime/container-runtime/src/containerRuntime.ts

@@ -2379,17 +2380,8 @@ export class ContainerRuntime extends TypedEventEmitter<IContainerRuntimeEvents>

            if (latestSnapshotRefSeq > this.deltaManager.lastSequenceNumber) {


nit: Remove this check since its done in waitForDeltaManagerToCatchup #Resolved

agarwal-navin · 2022-10-26T21:30:08Z

packages/runtime/container-runtime/src/containerRuntime.ts

@@ -2850,24 +2842,61 @@ export class ContainerRuntime extends TypedEventEmitter<IContainerRuntimeEvents>
        }
    }

+    private async waitForDeltaManagerToCatchup(latestSnapshotRefSeq: number,


nit: formatting of params should be consistent. #Resolved

agarwal-navin · 2022-10-26T21:30:43Z

packages/runtime/container-runtime/src/containerRuntime.ts

+        summaryLogger: ITelemetryLogger,
+        ): Promise<void> {
+        if (latestSnapshotRefSeq > this.deltaManager.lastSequenceNumber) {
+            // We need to catch up to the latest summary's reference sequence number before pausing.


Update this comment - it's not before pausing anymore since this is a generic function. #Resolved

agarwal-navin · 2022-10-26T21:32:14Z

packages/runtime/container-runtime/src/containerRuntime.ts

    /** Implementation of ISummarizerInternalsProvider.refreshLatestSummaryAck */
    public async refreshLatestSummaryAck(options: IRefreshSummaryAckOptions) {
        const { proposalHandle, ackHandle, summaryRefSeq, summaryLogger } = options;
        const readAndParseBlob = async <T>(id: string) => readAndParse<T>(this.storage, id);
        // The call to fetch the snapshot is very expensive and not always needed.
        // It should only be done by the summarizerNode, if required.
        const snapshotTreeFetcher = async () => {
+            // When fetching from storage we will always get the latest version and do not use the ackHandle.


nit: Move this comment outside this function with the outer comment. #Resolved

agarwal-navin · 2022-10-26T21:32:47Z

packages/runtime/container-runtime/src/containerRuntime.ts

+
+            const latestSnapshotRefSeq = await seqFromTree(fetchResult.snapshotTree, readAndParseBlob);
+            assert(latestSnapshotRefSeq >= summaryRefSeq,
+                    "Latest snapshot reference number greater than summaryRefSeq");


Suggested change

"Latest snapshot reference number greater than summaryRefSeq");

"Latest snapshot reference number should be greater than summaryRefSeq");

``` #Resolved

agarwal-navin · 2022-10-26T21:33:37Z

packages/runtime/container-runtime/src/containerRuntime.ts

+            {
+                eventName: "LatestSummaryRetrieved",
+                lastSequenceNumber: latestSnapshotRefSeq,
+                targetSequenceNumber: summaryRefSeq,


Should we also log the ackHandle so that it helps correlating with which ack this is done for? #Resolved

Might be good.

agarwal-navin · 2022-10-26T22:56:10Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+        return container;
+    };
+
+    it("Summarizer does not fetch during first summary", async () => {


What scenario is this testing? Summaries do not result in fetch right? #Resolved

agarwal-navin · 2022-10-26T22:57:20Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+
+        const summarizer2 = await createSummarizer(provider, mainContainer);
+        versionWrap = await incrementCellValueAndRunSummary(summarizer2, 4 /* expectedMatrixCellValue */);
+        assert(versionWrap.fetchCount === 0, "There should be one fetch after second summary");


This message says there should be one fetch but the assert is that fetchCount should be 0? #Resolved

agarwal-navin · 2022-10-26T22:58:59Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+
+        const containerRuntime = (summarizer as any).runtime as ContainerRuntime;
+        let getVersionsFunc = containerRuntime.storage.getVersions;
+        const funcSummary1 = async (versionId: string | null,


nit: Rename this to getVersionsOverride so that its clear what it is doing. #Resolved

agarwal-navin · 2022-10-26T23:00:01Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+        const value = getAndIncrementCellValue(mainDataStore.matrix, 0, 0, "1");
+        assert(value === 3, "Value matches expected");
+
+        const summarizer2 = await createSummarizer(provider, mainContainer);


There is no guarantee that this will load from latest summary. It can also load from the first summary created when the detached container attaches. #Resolved

agarwal-navin · 2022-10-26T23:08:35Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+        secondSummarizer.close();
+
+        // Load summarizer from previous version triggers fetch.
+         const newContainer = await loadContainer(summaryVersion);


Why create a container as well as summarizer? Why not just the summarizer? #Resolved

No particular reason - trying to make it a little different.

agarwal-navin · 2022-10-26T23:09:33Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+        assert(summaryVersion, "Summary version should be defined");
+        summarizerClient.close();
+
+        const secondSummarizer = await createSummarizer(provider, mainContainer);


nit: Add a comment that explains why couple more summaries are done, i.e., so that there are more recent summaries. #Resolved

agarwal-navin · 2022-10-26T23:11:10Z

packages/test/test-end-to-end-tests/src/test/SummarizeFetchValidation.spec.ts

+};
+export const TestDataObjectType1 = "@fluid-example/test-dataStore1";
+
+class TestDataObject1 extends DataObject {


I would suggest using the default data object provided by the TestObjectProvider. It has a root map that you can use to send ops. That will make the test simpler and also you don't have to worry about back-compat as it will be tested automatically. Here, the data store runtime will always be of the current version (N) and you lose compat testing. #Pending

AS I plan to add additional tests, it will give me more flexibility to do that.

agarwal-navin · 2022-10-26T23:14:12Z

packages/test/test-end-to-end-tests/src/test/SummarizeFromLatest.spec.ts

+
+        // Wait for the summarizer to run again.
+        const summaryVersion3 = await waitForSummary(summarizerClient2);
+        assert(summaryVersion3, "Summary version should be defined");


Is it possible to validate that the fetch was for the latest snapshot? #Pending

I could not find an easy way - will proceed with this check in so unblock Jatin and take a look at it later on.

agarwal-navin · 2022-10-26T23:15:27Z

packages/test/test-end-to-end-tests/src/test/SummarizeFromLatest.spec.ts

+        return container;
+    };
+
+    it("Loading summarizer from old snapshot", async () => {


It's not clear what this test is doing and its hard to follow. Can you please update the test name to reflect it? And also add either a description here or add comments in the test that explain what is happening. #Resolved

I was looking at this test and I realized I do not need it anymore - the FetchValidation does what I need.

agarwal-navin · 2022-10-26T23:15:58Z

packages/test/test-end-to-end-tests/src/test/SummarizeFromLatest.spec.ts

@@ -0,0 +1,224 @@
+/*!


Why is a separate file needed for this single test? Can't it be part of the above file? #Resolved

Removed the Latest as it does not add much value anymore.

agarwal-navin · 2022-10-26T23:17:46Z

packages/runtime/container-runtime/src/containerRuntime.ts

    /** Implementation of ISummarizerInternalsProvider.refreshLatestSummaryAck */
    public async refreshLatestSummaryAck(options: IRefreshSummaryAckOptions) {
        const { proposalHandle, ackHandle, summaryRefSeq, summaryLogger } = options;
        const readAndParseBlob = async <T>(id: string) => readAndParse<T>(this.storage, id);
        // The call to fetch the snapshot is very expensive and not always needed.
        // It should only be done by the summarizerNode, if required.
+        // When fetching from storage we will always get the latest version and do not use the ackHandle.


I still think we should handle scenarios where this can be called when a refresh is already in progress. We should not rely on the caller doing the right thing.
At least, add asserts or telemetry when this happens so we know someone screwed up. The telemetry will tell us if our assumption is wrong. #Resolved

Don't we have a lock in place for this not to happen ?

yeah but the lock is in summarizer classes. What if there is a bug and it ends up calling it multiple times. Adding an assert or log will help identify these scenarios rather than trying to figure it from other logs which has turned out to be quite cumbersome.

vladsud · 2022-11-02T16:44:36Z

packages/runtime/container-runtime/src/containerRuntime.ts

@@ -3091,11 +3117,16 @@ const waitForSeq = async (
    // TODO: remove cast to any when actual event is determined
    deltaManager.on("closed" as any, reject);


@kian-thompson - FYI, as you work on dispose/close on Container, we should have guidance for developers how to change their code, and ideally scan repo and fix our usage, if usage needs to be changed. I think there is no changes, but let's double check :)

…reference sequence number (#14052) ## Bug When refreshLatestSummaryAck is called, if pending state corresponding to the ack is not found, the latest summary is downloaded. This change was made fairly recently - [Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary by NicholasCouri · Pull Request #12313 · microsoft/FluidFramework (github.com)](#12313). This means that the snapshot that is downloaded may have a reference sequence number (fetchedSummaryRefSeq) that is greater than the ack's (summaryRefSeq). However, the latest summary reference sequence number of summarizer nodes and GC is updated to summaryRefSeq which can lead to inconsitent state and future summaries may be incorrect. ## Fix After the latest snapshot is downloaded, use the reference sequence number of this snapshot to update the state of the summarizer nodes and GC. ## Telemetry Improvements Made a couple of improvements to the refresh latest summary telemetry: - Renamed the telemetry to distinguish between refreshLatestSummaryAck and refreshLatestSummaryAckFromServer. This will help in analyzing logs. - Added the reference sequence number and version of the downloaded summary which was missing when snapshot was downloaded for refreshLatestSummaryAckFromServer.

…changes) Fixed bug - RefreshLatestSummaryAck can end up updating wrong latest reference sequence number (microsoft#14052) When refreshLatestSummaryAck is called, if pending state corresponding to the ack is not found, the latest summary is downloaded. This change was made fairly recently - [Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary by NicholasCouri · Pull Request microsoft#12313 · microsoft/FluidFramework (github.com)](microsoft#12313). This means that the snapshot that is downloaded may have a reference sequence number (fetchedSummaryRefSeq) that is greater than the ack's (summaryRefSeq). However, the latest summary reference sequence number of summarizer nodes and GC is updated to summaryRefSeq which can lead to inconsitent state and future summaries may be incorrect. After the latest snapshot is downloaded, use the reference sequence number of this snapshot to update the state of the summarizer nodes and GC. Made a couple of improvements to the refresh latest summary telemetry: - Renamed the telemetry to distinguish between refreshLatestSummaryAck and refreshLatestSummaryAckFromServer. This will help in analyzing logs. - Added the reference sequence number and version of the downloaded summary which was missing when snapshot was downloaded for refreshLatestSummaryAckFromServer.

NicholasCouri added 3 commits September 14, 2022 10:35

Remove generic uploadSummaryWIthContext.

9c10674

Attempt to always retrieve the latest summary

b64a1eb

Merge branch 'main' into Summary_GetLatest

6e9ee37

# Conflicts: # packages/runtime/container-runtime/src/containerRuntime.ts

NicholasCouri requested review from agarwal-navin, vladsud and andre4i October 6, 2022 18:51

NicholasCouri requested review from a team as code owners October 6, 2022 18:51

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 Oct 6, 2022

agarwal-navin reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

agarwal-navin reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 6, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Show resolved Hide resolved

Creating a waitforCatchup method and run before refreshing the summar…

35fda43

…izer nodes.

NicholasCouri added 3 commits October 11, 2022 17:07

Merge branch 'main' into Summary_GetLatest

4d939d3

Remove provider from test code and replace with FluidObject.

7be13a7

Merge branch 'main' into Summary_GetLatest

5f285c4

vladsud reviewed Oct 14, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 14, 2022

View reviewed changes

packages/runtime/container-runtime/src/containerRuntime.ts Outdated Show resolved Hide resolved

vladsud reviewed Oct 14, 2022

View reviewed changes

packages/test/test-end-to-end-tests/src/test/SummarizeFromLatest.spec.ts Outdated Show resolved Hide resolved

NicholasCouri added 4 commits October 21, 2022 14:00

Adding new tests to validate fetch from storage calls.

c4fe6ec

Addressing review comments

e95aec8

Added missing updated tests.

0130c91

Remove unused ReadAndParseBlob

35b9c66

agarwal-navin reviewed Oct 26, 2022

View reviewed changes

Addressing latest review comments.

5ef847e

agarwal-navin reviewed Oct 26, 2022

View reviewed changes

agarwal-navin approved these changes Oct 26, 2022

View reviewed changes

Addressing latest comments.

5f43213

NicholasCouri merged commit e28fb39 into microsoft:main Nov 1, 2022

vladsud reviewed Nov 2, 2022

View reviewed changes

agarwal-navin mentioned this pull request Feb 7, 2023

Fixed bug - RefreshLatestSummaryAck can end up updating wrong latest reference sequence number #14052

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary #12313

Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary #12313

NicholasCouri commented Oct 6, 2022

msfluid-bot commented Oct 6, 2022 •

edited

Loading

vladsud commented Oct 7, 2022

vladsud commented Oct 7, 2022

NicholasCouri commented Oct 7, 2022 •

edited

Loading

NicholasCouri commented Oct 7, 2022

vladsud commented Oct 14, 2022

NicholasCouri commented Oct 14, 2022 •

edited

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 26, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 31, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Nov 1, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 31, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 31, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 31, 2022

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

NicholasCouri Oct 31, 2022

agarwal-navin Nov 2, 2022

vladsud Nov 2, 2022

		@@ -2379,17 +2380,8 @@ export class ContainerRuntime extends TypedEventEmitter<IContainerRuntimeEvents>

		if (latestSnapshotRefSeq > this.deltaManager.lastSequenceNumber) {

	"Latest snapshot reference number greater than summaryRefSeq");
	"Latest snapshot reference number should be greater than summaryRefSeq");
	``` #Resolved

		@@ -3091,11 +3117,16 @@ const waitForSeq = async (
		// TODO: remove cast to any when actual event is determined
		deltaManager.on("closed" as any, reject);

Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary #12313

Proposal - refreshLatestSummaryAck's fetchSnapshotFromStorage always retrieving the latest summary #12313

Conversation

NicholasCouri commented Oct 6, 2022

Description

msfluid-bot commented Oct 6, 2022 • edited Loading

vladsud commented Oct 7, 2022

vladsud commented Oct 7, 2022

NicholasCouri commented Oct 7, 2022 • edited Loading

NicholasCouri commented Oct 7, 2022

vladsud commented Oct 14, 2022

NicholasCouri commented Oct 14, 2022 • edited Loading

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agarwal-navin Oct 26, 2022 • edited by NicholasCouri Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msfluid-bot commented Oct 6, 2022 •

edited

Loading

NicholasCouri commented Oct 7, 2022 •

edited

Loading

NicholasCouri commented Oct 14, 2022 •

edited

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading

agarwal-navin Oct 26, 2022 •

edited by NicholasCouri

Loading