From b25431a56e06abc34723f54eafe4588e5c7dea8c Mon Sep 17 00:00:00 2001
From: Xinyu Ma <ma.xinyu.26a@kyoto-u.jp>
Date: Mon, 15 Apr 2024 09:36:14 -0700
Subject: [PATCH] Add more retry times to wait for the solution in
 SegmentFetcher

---
 package.json                 |  2 +-
 src/sync-agent/deliveries.ts | 12 ++++++++++--
 src/sync-agent/sync-agent.ts | 11 ++++++++++-
 src/workspace/workspace.ts   |  7 ++-----
 4 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/package.json b/package.json
index 8452be5..208aec1 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@ucla-irl/ndnts-aux",
-  "version": "2.0.2",
+  "version": "2.0.3",
   "description": "NDNts Auxiliary Package for Web and Deno",
   "scripts": {
     "test": "deno test --no-check",
diff --git a/src/sync-agent/deliveries.ts b/src/sync-agent/deliveries.ts
index e97cd83..7735952 100644
--- a/src/sync-agent/deliveries.ts
+++ b/src/sync-agent/deliveries.ts
@@ -243,12 +243,20 @@ export class AtLeastOnceDelivery extends SyncDelivery {
     const continuation = fetchSegments(prefix, {
       segmentNumConvention: SequenceNum,
       segmentRange: [update.loSeqNum, update.hiSeqNum + 1],
-      retxLimit: 80,
+      retxLimit: 600,
       lifetimeAfterRto: 1000, // Lifetime = RTO + 1000
       // The true timeout timer is the RTO, specified below
       rtte: {
         initRto: 50,
-        minRto: 50, // Minimal RTO is 50ms
+        // Minimal RTO is 50ms
+        // Note: This has to be small enough to quickly trigger the path switch of BestRoute strategy
+        // However, due to an implementation flaw of SegmentFetcher, RTO only doubles once per window.
+        // Thus, maxRTO does not make any sense most of the time: the RTT in Workspace can only be very small
+        // (when Interest is forwarded to right path) or very large (when wrong path or Data missing).
+        // Therefore, before the SegmentFetcher issue can get resolved, we use the retxLimit to control the
+        // maximum timeout: retxLimit = Expected Timeout / (2 * minRto)
+        // Default expected timeout is 1 min, thus retxLimit == 600
+        minRto: 50,
         maxRto: 2000,
       },
       ca: new LimitedCwnd(new TcpCubic(), 10),
diff --git a/src/sync-agent/sync-agent.ts b/src/sync-agent/sync-agent.ts
index ffde552..3c6a2b5 100644
--- a/src/sync-agent/sync-agent.ts
+++ b/src/sync-agent/sync-agent.ts
@@ -79,7 +79,16 @@ export class SyncAgent implements AsyncDisposable {
     return this._ready;
   }
 
+  /**
+   * Set the readiness to start or stop processing updates.
+   * Condition: Sync deliveries are only running when ready is true.
+   */
   public set ready(value: boolean) {
+    if (this._ready && !value) {
+      throw new Error(
+        'Due to implementation limit SVS is unstoppable. Please do not set ready = false manually for now.',
+      );
+    }
     this._ready = value;
     if (value) {
       this.atLeastOnce.start();
@@ -201,7 +210,7 @@ export class SyncAgent implements AsyncDisposable {
         verifier: this.verifier,
         modifyInterest: { mustBeFresh: true },
         lifetimeAfterRto: 2000,
-        retxLimit: 25,
+        retxLimit: 150, // See Deliveries. 60*1000/(2*200)=150. Default minRto = 150.
       });
       for await (const segment of result) {
         // Cache packets
diff --git a/src/workspace/workspace.ts b/src/workspace/workspace.ts
index 64998da..864efb3 100644
--- a/src/workspace/workspace.ts
+++ b/src/workspace/workspace.ts
@@ -86,11 +86,8 @@ export class Workspace implements AsyncDisposable {
   }
 
   public async destroy() {
-    this.syncAgent.ready = false;
-    await Promise.all([
-      this.yjsSnapshotMgr.destroy(),
-      this.syncAgent.destroy(),
-    ]);
+    await this.syncAgent.destroy();
+    await this.yjsSnapshotMgr.destroy();
     // persistStore is not created by workspace
   }