PR comments integration

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

@@ -252,10 +252,6 @@ export abstract class FluidDataStoreContext extends TypedEventEmitter<IFluidData
             async () => this.getGCDataInternal(),
             async () => this.getInitialGCSummaryDetails(),
         );
-
-        // Add self route (empty string) to used routes in the summarizer node. If GC is enabled, the used routes will
-        // be updated as per the GC data.
-        this.summarizerNode.updateUsedRoutes([""]);
     }
 
     public dispose(): void {
@@ -471,14 +467,15 @@ export abstract class FluidDataStoreContext extends TypedEventEmitter<IFluidData
      * Calls the channel to update used routes of its child contexts.
      */
     private updateChannelUsedRoutes() {
-        assert(this.channel !== undefined, "Channel should not be undefined when updating used routes");
-        // Remove the route to this data store, if it exists.
-        const usedChannelRoutes = this.summarizerNode.usedRoutes.filter(
-            (id: string) => { return id !== "/" && id !== ""; },
-        );
+        assert(this.loaded, "Channel should be loaded when updating used routes");
+        assert(this.channel !== undefined, "Channel should be present when data store is loaded");
 
         // back-compat: 0.33 - updateUsedRoutes is added in 0.33. Remove the check here when N >= 0.36.
         if (this.channel.updateUsedRoutes !== undefined) {
+            // Remove the route to this data store, if it exists.
+            const usedChannelRoutes = this.summarizerNode.usedRoutes.filter(
+                (id: string) => { return id !== "/" && id !== ""; },
+            );
             this.channel.updateUsedRoutes(usedChannelRoutes);
         }
     }

packages/runtime/datastore/src/remoteChannelContext.ts

@@ -90,13 +90,6 @@ export class RemoteChannelContext implements IChannelContext {
             async () => this.getGCDataInternal(),
             async () => this.gcDetailsInInitialSummaryP,
         );
-
-        // Add self route (empty string) to used routes in the summarizer node. If GC is enabled, the used routes will
-        // be updated as per the GC data.
-        // back-compat: 0.33 - updateUsedRoutes is added in 0.33. Remove the check here when N >= 0.36.
-        if (this.summarizerNode.updateUsedRoutes !== undefined) {
-            this.summarizerNode.updateUsedRoutes([""]);
-        }
     }
 
     // eslint-disable-next-line @typescript-eslint/promise-function-async

packages/runtime/runtime-utils/src/summarizerNode/summarizerNodeWithGc.ts

@@ -64,7 +64,10 @@ export class SummarizerNodeWithGC extends SummarizerNode implements IRootSummari
     // The GC details of this node in the initial summary.
     private readonly gcDetailsInInitialSummaryP: LazyPromise<IGarbageCollectionSummaryDetails>;
 
-    private _usedRoutes: string[] = [];
+    // Set used routes to have self route by default. This makes the node referenced by default. This is done to ensure
+    // that this node is not marked as collected when running GC has been disabled. Once, the option to disable GC is
+    // removed (from runGC flag in IContainerRuntimeOptions), this should be changed to be have no routes by default.
+    private _usedRoutes: string[] = [""];
     public get usedRoutes(): string[] {
         return this._usedRoutes;
     }

packages/runtime/runtime-utils/src/test/summarizerNodeWithGc.spec.ts

@@ -109,7 +109,7 @@ describe("SummarizerNodeWithGC Tests", () => {
         it("can return initial GC data when nothing has changed since last summary", async () => {
             // Set the data to be returned by getinitialGCSummaryDetails.
             initialGCSummaryDetails = {
-                usedRoutes: [],
+                usedRoutes: [""],
                 gcData: {
                     gcNodes: {
                         "/": [ node1Id ],
@@ -129,7 +129,7 @@ describe("SummarizerNodeWithGC Tests", () => {
             // Set initial GC data to undefined. This will force the summarizer node to generate GC data even though
             // nothing changed since last summary.
             initialGCSummaryDetails = {
-                usedRoutes: [],
+                usedRoutes: [""],
                 gcData: undefined,
             };
 
@@ -141,7 +141,7 @@ describe("SummarizerNodeWithGC Tests", () => {
             // Set initial GC data to undefined. This will force the summarizer node to generate GC data even though
             // nothing changed since last summary.
             initialGCSummaryDetails = {
-                usedRoutes: [],
+                usedRoutes: [""],
                 gcData: undefined,
             };
 
@@ -187,7 +187,7 @@ describe("SummarizerNodeWithGC Tests", () => {
             // Set initial GC data to undefined. This will force the summarizer node to generate GC data even though
             // nothing changed since last summary.
             initialGCSummaryDetails = {
-                usedRoutes: [],
+                usedRoutes: [""],
                 gcData: undefined,
             };
 
@@ -217,10 +217,10 @@ describe("SummarizerNodeWithGC Tests", () => {
         });
 
         it("can return empty GC data when summarizing without generating GC data", async () => {
-            // Set initial used routes to empty. Since the summarizer node's default used routes is also empty, this
-            // ensures that used routes is unchanged.
+            // Set initial used routes to self route. Since the summarizer node's default used routes is also this, it
+            // will ensure that used routes is unchanged.
             initialGCSummaryDetails = {
-                usedRoutes: [],
+                usedRoutes: [""],
             };
 
             // Call summarize with fullTree as false. This should try to get cached GC data. But since no GC data was
@@ -247,10 +247,10 @@ describe("SummarizerNodeWithGC Tests", () => {
         });
 
         it("can return GC data when used routes changed since last summary", async () => {
-            // Set initial used routes to have some value. This will force the summarizer to generate summary again
-            // because reference used route will be different from summarizer node's default used routes (empty).
+            // Set initial used routes to be empty. This will force the summarizer to generate summary again
+            // because reference used route will be different from summarizer node's default used routes (self route).
             initialGCSummaryDetails = {
-                usedRoutes: [""],
+                usedRoutes: [],
             };
 
             // Call getGCData to generate GC data. This will generate GC data even though nothing changes since initial