Allow a callback to be passed to cleanDeletedBlocks making it interru…

…ptible. (#6842) Co-authored-by: Ben Polinsky <[email protected]> Co-authored-by: imodeljs-admin <[email protected]> Co-authored-by: Paul Connelly <[email protected]>
iTwin · Jun 14, 2024 · c1d8e09 · c1d8e09
1 parent 74d22cd
commit c1d8e09
Show file tree

Hide file tree

Showing 12 changed files with 204 additions and 26 deletions.
diff --git a/common/api/core-backend.api.md b/common/api/core-backend.api.md
@@ -953,10 +953,13 @@ export namespace CloudSqlite {
         name: string;
         rootDir: string;
     }
+    export function cleanDeletedBlocks(container: CloudContainer, options: CleanDeletedBlocksOptions): Promise<void>;
     // (undocumented)
     export interface CleanDeletedBlocksOptions {
         debugLogging?: boolean;
+        findOrphanedBlocks?: boolean;
         nSeconds?: number;
+        onProgress?: (nDeleted: number, nTotalToDelete: number) => number;
     }
     export interface CloudCache {
         // @internal
@@ -986,7 +989,6 @@ export namespace CloudSqlite {
         // (undocumented)
         readonly cache?: CloudCache;
         checkForChanges(): void;
-        cleanDeletedBlocks(options?: CleanDeletedBlocksOptions): Promise<void>;
         clearWriteLock(): void;
         connect(cache: CloudCache): void;
         get containerId(): string;

diff --git a/common/changes/@itwin/core-backend/nick-cloudsqlite-cleanup-callback_2024-06-13-16-10.json b/common/changes/@itwin/core-backend/nick-cloudsqlite-cleanup-callback_2024-06-13-16-10.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/core-backend",
+      "comment": "Make cleanDeletedBlocks interruptible, and remove function from CloudContainer object. Add it to CloudSqlite namespace",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/core-backend"
+}
diff --git a/...n/changes/@itwin/workspace-editor/nick-cloudsqlite-cleanup-callback_2024-06-13-16-10.json b/...n/changes/@itwin/workspace-editor/nick-cloudsqlite-cleanup-callback_2024-06-13-16-10.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/workspace-editor",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/workspace-editor"
+}
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git a/core/backend/package.json b/core/backend/package.json
@@ -98,7 +98,7 @@
     "webpack": "^5.76.0"
   },
   "dependencies": {
-    "@bentley/imodeljs-native": "4.8.2",
+    "@bentley/imodeljs-native": "4.8.4",
     "@itwin/cloud-agnostic-core": "^2.1.0",
     "@itwin/core-telemetry": "workspace:*",
     "@itwin/object-storage-azure": "^2.2.2",

diff --git a/core/backend/src/CloudSqlite.ts b/core/backend/src/CloudSqlite.ts
@@ -360,8 +360,23 @@ export namespace CloudSqlite {
      * a 404 error. Default is 1 hour.
      */
     nSeconds?: number;
-    /** if enabled, outputs verbose logs about the cleanup process. These would include outputting blocks which are determined as eligible for deletion. */
+    /** if enabled, outputs verbose logs about the cleanup process. Output includes blocks determined eligible for deletion.
+     * @default false
+    */
     debugLogging?: boolean;
+    /** If true, iterates over all blobs in the cloud container to add blocks that are 'orphaned' to the delete list in the manifest.
+     * Orphaned blocks are created when a client abruptly halts, is disconnected, or encounters an error while uploading a change.
+     * If false, the search for 'orphaned' blocks is skipped and only any blocks which are already on the delete list are deleted.
+     * @default true
+     */
+    findOrphanedBlocks?: boolean;
+    /**
+     * a user-supplied progress function called during the cleanup operation. While the search for orphaned blocks occurs, nDeleted will be 0 and nTotalToDelete will be 1.
+     * Once the search is complete and orphaned blocks begin being deleted, nDeleted will be the number of blocks deleted and nTotalToDelete will be the total number of blocks to delete.
+     * If the return value is 1, the job will be cancelled and progress will be saved. If one or more blocks have already been deleted, then a new manifest file is uploaded saving the progress of the delete job.
+     * Return any other non-0 value to cancel the job without saving progress.
+     */
+    onProgress?: (nDeleted: number, nTotalToDelete: number) => number;
   }
 
   /**
@@ -498,15 +513,6 @@ export namespace CloudSqlite {
      */
     uploadChanges(): Promise<void>;
 
-    /**
-     * Clean any unused deleted blocks from cloud storage. When a database is written, a subset of its blocks are replaced
-     * by new versions, sometimes leaving the originals unused. In this case, they are not deleted immediately.
-     * Instead, they are scheduled for deletion at some later time. Calling this method deletes all blocks in the cloud container
-     * for which the scheduled deletion time has passed.
-     * @param options options which influence the behavior of cleanDeletedBlocks. @see CleanDeletedBlocksOptions
-     */
-    cleanDeletedBlocks(options?: CleanDeletedBlocksOptions): Promise<void>;
-
     /**
      * Create a copy of an existing database within this CloudContainer with a new name.
      * @note CloudSqlite uses copy-on-write semantics for this operation. That is, this method merely makes a
@@ -515,8 +521,8 @@ export namespace CloudSqlite {
      */
     copyDatabase(dbName: string, toAlias: string): Promise<void>;
 
-    /** Remove a database from this CloudContainer.
-     * @see cleanDeletedBlocks
+    /** Remove a database from this CloudContainer. Unused blocks are moved to the delete list in the manifest.
+     * @see [[CloudSqlite.cleanDeletedBlocks]] to actually delete the blocks from the delete list.
      */
     deleteDatabase(dbName: string): Promise<void>;
 
@@ -576,14 +582,54 @@ export namespace CloudSqlite {
     promise: Promise<boolean>;
   }
 
+  /**
+   * Clean any unused deleted blocks from cloud storage. Unused deleted blocks can accumulate in cloud storage in a couple of ways:
+   * 1) When a database is updated, a subset of its blocks are replaced by new versions, sometimes leaving the originals unused.
+   * 2) A database is deleted with [[CloudContainer.deleteDatabase]]
+   * In both cases, the blocks are not deleted immediately. Instead, they are scheduled for deletion at some later time.
+   * Calling this method deletes all blocks in the cloud container for which the scheduled deletion time has passed.
+   * @param container the CloudContainer to be cleaned. Must be connected and hold the write lock.
+   * @param options options for the cleanup operation. @see CloudSqlite.CleanDeletedBlocksOptions
+   */
+  export async function cleanDeletedBlocks(container: CloudContainer, options: CleanDeletedBlocksOptions): Promise<void> {
+    let timer: NodeJS.Timeout | undefined;
+    try {
+      const cleanJob = new NativeLibrary.nativeLib.CancellableCloudSqliteJob("cleanup", container, options);
+      let total = 0;
+      const onProgress = options?.onProgress;
+      if (onProgress) {
+        timer = setInterval(async () => { // set an interval timer to show progress every 250ms
+          const progress = cleanJob.getProgress();
+          total = progress.total;
+          const result = onProgress(progress.loaded, progress.total);
+          if (result === 1)
+            cleanJob.stopAndSaveProgress();
+          else if (result !== 0)
+            cleanJob.cancelTransfer();
+        }, 250);
+      }
+      await cleanJob.promise;
+      onProgress?.(total, total); // make sure we call progress func one last time when download completes
+      container.checkForChanges(); // re-read the manifest so the number of garbage blocks is updated.
+    } catch (err: any) {
+      if (err.message === "cancelled")
+        err.errorNumber = BriefcaseStatus.DownloadCancelled;
+
+      throw err;
+    } finally {
+      if (timer)
+        clearInterval(timer);
+    }
+  }
+
   /** @internal */
   export async function transferDb(direction: TransferDirection, container: CloudContainer, props: TransferDbProps) {
     if (direction === "download")
       mkdirSync(dirname(props.localFileName), { recursive: true }); // make sure the directory exists before starting download
 
     let timer: NodeJS.Timeout | undefined;
     try {
-      const transfer = new NativeLibrary.nativeLib.CloudDbTransfer(direction, container, props);
+      const transfer = new NativeLibrary.nativeLib.CancellableCloudSqliteJob(direction, container, props);
       let total = 0;
       const onProgress = props.onProgress;
       if (onProgress) {
@@ -604,6 +650,7 @@ export namespace CloudSqlite {
     } finally {
       if (timer)
         clearInterval(timer);
+
     }
   }
 

diff --git a/full-stack-tests/backend/src/integration/AzuriteTest.ts b/full-stack-tests/backend/src/integration/AzuriteTest.ts
@@ -10,6 +10,7 @@ import * as azureBlob from "@azure/storage-blob";
 import { BlobContainer, CloudSqlite, IModelHost, SettingsContainer } from "@itwin/core-backend";
 import { AccessToken, Guid } from "@itwin/core-bentley";
 import { LocalDirName, LocalFileName } from "@itwin/core-common";
+import * as crypto from "crypto";
 
 // spell:ignore imodelid itwinid mkdirs devstoreaccount racwdl
 
@@ -33,6 +34,25 @@ export namespace AzuriteTest {
   export namespace Sqlite {
     export type TestContainer = CloudSqlite.CloudContainer;
 
+    export const uploadDummyBlock = async (container: CloudSqlite.CloudContainer, blockNameSize: number): Promise<string> => {
+      const generateRandomHexString = (length: number) => {
+        const bytes = crypto.randomBytes(length);
+        const hexString = bytes.toString("hex");
+        return hexString.toUpperCase();
+      };
+      const azClient = createAzClient(container.containerId);
+      const blockName = `${generateRandomHexString(blockNameSize)}.bcv`;
+      const blobClient = azClient.getBlockBlobClient(blockName);
+      await blobClient.uploadData(Buffer.alloc(0));
+      return blockName;
+    };
+
+    export const checkBlockExists = async (container: CloudSqlite.CloudContainer, blockName: string): Promise<boolean> => {
+      const azClient = createAzClient(container.containerId);
+      const blobClient = azClient.getBlockBlobClient(blockName);
+      return blobClient.exists();
+    };
+
     export const setSasToken = async (container: CloudSqlite.CloudContainer, accessLevel: BlobContainer.RequestAccessLevel) => {
       container.accessToken = await CloudSqlite.requestToken({ baseUri, containerId: container.containerId, accessLevel });
     };

diff --git a/full-stack-tests/backend/src/integration/CloudSqlite.test.ts b/full-stack-tests/backend/src/integration/CloudSqlite.test.ts
@@ -8,7 +8,7 @@ import * as chaiAsPromised from "chai-as-promised";
 import { existsSync, removeSync } from "fs-extra";
 import { join } from "path";
 import * as sinon from "sinon";
-import { BlobContainer, BriefcaseDb, CloudSqlite, IModelHost, KnownLocations, PropertyStore, SnapshotDb, SQLiteDb } from "@itwin/core-backend";
+import { BlobContainer, BriefcaseDb, CloudSqlite, IModelHost, IModelJsFs, KnownLocations, PropertyStore, SnapshotDb, SQLiteDb } from "@itwin/core-backend";
 import { KnownTestLocations } from "@itwin/core-backend/lib/cjs/test";
 import { assert, BeDuration, DbResult, Guid, GuidString, OpenMode } from "@itwin/core-bentley";
 import { AzuriteTest } from "./AzuriteTest";
@@ -335,14 +335,15 @@ describe("CloudSqlite", () => {
     await azSqlite.setSasToken(contain1, "read"); // don't ask for delete permission
     contain1.connect(caches[1]);
     await CloudSqlite.withWriteLock({ user: user1, container: contain1 }, async () => {
-      await expect(contain1.cleanDeletedBlocks()).eventually.rejectedWith("not authorized").property("errorNumber", 403);
+      // need nSeconds 0 or the blocks of the database we just deleted won't be deleted.
+      await expect(CloudSqlite.cleanDeletedBlocks(contain1, { nSeconds: 0 })).eventually.rejectedWith("delete block failed (403)");
     });
 
     contain1.disconnect();
     await azSqlite.setSasToken(contain1, "admin"); // now ask for delete permission
     contain1.connect(caches[1]);
 
-    await CloudSqlite.withWriteLock({ user: user1, container: contain1 }, async () => contain1.cleanDeletedBlocks());
+    await CloudSqlite.withWriteLock({ user: user1, container: contain1 }, async () => CloudSqlite.cleanDeletedBlocks(contain1, {nSeconds: 0}));
     expect(contain1.garbageBlocks).equals(0); // should successfully purge
 
     // should be connected
@@ -445,6 +446,94 @@ describe("CloudSqlite", () => {
     newCache2.destroy();
   });
 
+  it("should be able to interrupt cleanDeletedBlocks operation", async () => {
+    const cache = azSqlite.makeCache("clean-blocks-cache");
+    const container = testContainers[0];
+
+    const dbName = "testBimForCleaningBlocks";
+
+    const pathToCopy = join(KnownLocations.tmpdir, `${dbName}`);
+
+    if (IModelJsFs.existsSync(pathToCopy))
+      IModelJsFs.removeSync(pathToCopy);
+    const sqliteDb = new SQLiteDb();
+    sqliteDb.createDb(pathToCopy, undefined, { rawSQLite: true });
+    sqliteDb.executeSQL("CREATE TABLE TestData(id INTEGER PRIMARY KEY,val BLOB)");
+    // Insert 16mb so we have some data to delete.
+    sqliteDb.executeSQL(`INSERT INTO TestData(id,val) VALUES (1, randomblob(1024*1024*16))`);
+
+    sqliteDb.saveChanges();
+    sqliteDb.closeDb();
+    await azSqlite.setSasToken(container, "write"); // get a write token to be able to upload.
+    await azSqlite.uploadFile(container, cache, dbName, pathToCopy);
+
+    container.connect(cache);
+    const dbs = container.queryDatabases();
+    expect(dbs.length).to.be.greaterThanOrEqual(1);
+    const db = container.queryDatabase(dbName);
+    expect(db !== undefined).to.be.true;
+
+    expect(container.garbageBlocks).to.be.equal(0);
+    container.acquireWriteLock("testuser");
+    await container.deleteDatabase(dbName);
+    await container.uploadChanges();
+    expect(container.queryDatabase(dbName)).to.be.undefined;
+
+    expect(container.garbageBlocks).to.be.greaterThan(0);
+    const garbageBlocksPrev = container.garbageBlocks;
+
+    // cleanDeletedBlocks defaults to an nSeconds of 3600, so we expect to keep our garbage blocks, because they are less than 3600 seconds old.
+    await CloudSqlite.cleanDeletedBlocks(container, {});
+    expect(container.garbageBlocks).to.be.equal(garbageBlocksPrev);
+
+    // Upload dummy block to simulate an orphaned block.
+    const blockName = await azSqlite.uploadDummyBlock(container, 24);
+    // findOrphanedBlocks is false, so we expect to keep our dummy block.
+    await CloudSqlite.cleanDeletedBlocks(container, {findOrphanedBlocks: false});
+    await expect(azSqlite.checkBlockExists(container, blockName)).to.eventually.become(true);
+    // findOrphanedBlocks is true, so we expect to remove our dummy block.
+    await CloudSqlite.cleanDeletedBlocks(container, {findOrphanedBlocks: true});
+    await expect(azSqlite.checkBlockExists(container, blockName)).to.eventually.become(false);
+
+    expect(container.garbageBlocks).to.be.equal(garbageBlocksPrev);
+
+    const onProgress = sinon.stub();
+    onProgress.onFirstCall().returns(2);
+
+    // Faking the interval setup in cleanDeletedBlocks.
+    const clock = sinon.useFakeTimers({toFake: ["setInterval"], shouldAdvanceTime: true, advanceTimeDelta: 1});
+    let resolved = false;
+    CloudSqlite.cleanDeletedBlocks(container, {nSeconds: 0, findOrphanedBlocks: true, onProgress}).then(() => {
+      resolved = true;
+    }).catch(() => {
+      resolved = true;
+    });
+
+    while (!resolved) {
+      await clock.tickAsync(250);
+      await new Promise((resolve) => clock.setTimeout(resolve, 1));
+    }
+    container.checkForChanges();
+
+    expect(onProgress.called).to.be.true;
+    // We aborted our cleanup, so expect no progress to be shown.
+    expect(container.garbageBlocks).to.be.equal(garbageBlocksPrev);
+
+    resolved = false;
+    clock.reset();
+    clock.restore();
+
+    onProgress.reset();
+    onProgress.returns(0);
+    // One final clean that we don't interrupt ( because we always return 0 from onProgress)
+    await CloudSqlite.cleanDeletedBlocks(container, {nSeconds: 0, onProgress});
+    container.checkForChanges();
+    expect(container.garbageBlocks).to.be.equal(0);
+
+    container.releaseWriteLock();
+    container.disconnect({detach: true});
+  });
+
   /** make sure that the auto-refresh for container tokens happens every hour */
   it("Auto refresh container tokens", async () => {
     const contain1 = testContainers[0];

diff --git a/test-apps/display-test-app/android/imodeljs-test-app/app/build.gradle b/test-apps/display-test-app/android/imodeljs-test-app/app/build.gradle
@@ -41,7 +41,7 @@ dependencies {
     implementation 'com.google.android.material:material:1.7.0'
     implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
     implementation 'androidx.navigation:navigation-ui:2.5.3'
-    implementation 'com.github.itwin:mobile-native-android:4.8.2'
+    implementation 'com.github.itwin:mobile-native-android:4.8.4'
     implementation 'androidx.webkit:webkit:1.5.0'
 }
 

diff --git a/test-apps/display-test-app/ios/imodeljs-test-app/imodeljs-test-app.xcodeproj/project.pbxproj b/test-apps/display-test-app/ios/imodeljs-test-app/imodeljs-test-app.xcodeproj/project.pbxproj
@@ -453,7 +453,7 @@
 			repositoryURL = "https://github.com/iTwin/mobile-native-ios";
 			requirement = {
 				kind = exactVersion;
-				version = 4.8.2;
+				version = 4.8.4;
 			};
 		};
 /* End XCRemoteSwiftPackageReference section */

diff --git a/tools/internal/ios/core-test-runner/core-test-runner.xcodeproj/project.pbxproj b/tools/internal/ios/core-test-runner/core-test-runner.xcodeproj/project.pbxproj
@@ -552,7 +552,7 @@
 			repositoryURL = "https://github.com/iTwin/mobile-native-ios";
 			requirement = {
 				kind = exactVersion;
-				version = 4.8.2;
+				version = 4.8.4;
 			};
 		};
 /* End XCRemoteSwiftPackageReference section */

diff --git a/utils/workspace-editor/src/WorkspaceEditor.ts b/utils/workspace-editor/src/WorkspaceEditor.ts
@@ -458,7 +458,7 @@ async function initializeWorkspace(args: InitializeOpts) {
 async function purgeWorkspace(args: EditorOpts) {
   const container = getCloudContainer(args);
   const nGarbage = container.garbageBlocks;
-  await CloudSqlite.withWriteLock({ ...args, container }, async () => container.cleanDeletedBlocks());
+  await CloudSqlite.withWriteLock({ ...args, container }, async () => CloudSqlite.cleanDeletedBlocks(container, {}));
   container.checkForChanges(); // re-read manifest to get current garbage count
   showMessage(`purged ${sayContainer(args)}. ${nGarbage - container.garbageBlocks} garbage blocks cleaned`);
 }