Merge remote-tracking branch 'origin/candidate-9.12.x' into candidate-9.14.x

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>

Author: GordonSmith

dali/base/dasds.cpp

@@ -1119,15 +1119,15 @@ class CTransactionItem : public CSimpleInterface
 
 static constexpr unsigned defaultSaveThresholdSecs = 0; // disabled
 static constexpr unsigned defaultDeltaSaveTransactionThreshold = 0; // disabled
-static constexpr unsigned defaultDeltaMemMaxMB = 10;
+static constexpr unsigned defaultDeltaMemMaxMB = 64; // 64MB a reasonably large default, to cope with large blobs being committed
 static constexpr unsigned defaultDeltaTransactionQueueLimit = 10000;
 class CDeltaWriter : implements IThreaded
 {
     IStoreHelper *iStoreHelper = nullptr;
     StringBuffer dataPath;
     StringBuffer backupPath;
     unsigned transactionQueueLimit = defaultDeltaTransactionQueueLimit; // absolute limit, will block if this far behind
-    memsize_t transactionMaxMem = defaultDeltaMemMaxMB * 0x100000; // 10MB
+    memsize_t transactionMaxMem = defaultDeltaMemMaxMB * 0x100000;
     unsigned totalQueueLimitHits = 0;
     unsigned saveThresholdSecs = 0;
     cycle_t lastSaveTime = 0;
@@ -1414,40 +1414,34 @@ class CDeltaWriter : implements IThreaded
                 // keep going whilst there's things pending
                 while (true)
                 {
-                    CLeavableCriticalBlock b(pendingCrit);
-                    std::queue<Owned<CTransactionItem>> todo = std::move(pending);
-                    if (0 == todo.size())
+                    // NB: ensure consistent lock ordering of blockedSaveCrit and pendingCrit
+                    CHECKEDCRITICALBLOCK(blockedSaveCrit, fakeCritTimeout); // because if Dali is saving state (::blockingSave), it will clear pending
+                    std::queue<Owned<CTransactionItem>> todo;
                     {
-                        if (writeRequested)
+                        CriticalBlock b(pendingCrit);
+                        todo = std::move(pending);
+                        if (0 == todo.size())
                         {
-                            // NB: if here, implies someone signalled via requestAsyncWrite()
+                            if (writeRequested)
+                            {
+                                // NB: if here, implies someone signalled via requestAsyncWrite()
 
-                            // if reason we're here is because sem timedout, consume the signal that was sent
-                            if (semTimedout)
-                                pendingTransactionsSem.wait();
+                                // if reason we're here is because sem timedout, consume the signal that was sent
+                                if (semTimedout)
+                                    pendingTransactionsSem.wait();
 
-                            writeRequested = false;
-                            if (signalWhenAllWritten) // can only be true if writeRequested was true
-                            {
-                                signalWhenAllWritten = false;
-                                allWrittenSem.signal();
+                                writeRequested = false;
+                                if (signalWhenAllWritten) // can only be true if writeRequested was true
+                                {
+                                    signalWhenAllWritten = false;
+                                    allWrittenSem.signal();
+                                }
                             }
+                            break;
                         }
-                        break;
+                        pendingSz = 0;
                     }
-                    pendingSz = 0;
 
-                    b.leave(); // NB: addToQueue could add to pending between here and regaining lock below
-                    // NB: ensure consistent lock ordering of blockedSaveCrit and pendingCrit
-                    CHECKEDCRITICALBLOCK(blockedSaveCrit, fakeCritTimeout); // because if Dali is saving state (::blockingSave), it will clear pending
-                    b.enter();
-                    // check if new items added in window above
-                    while (pending.size())
-                    {
-                        todo.push(std::move(pending.front()));
-                        pending.pop();
-                    }
-                    b.leave();
                     // Because blockedSaveCrit is held, it will also block 'synchronous save' (see addToQueue)
                     // i.e. if stuck here, the transactions will start building up, and trigger a 'Forced synchronous save',
                     // which will in turn block. This must complete!
@@ -2266,14 +2260,15 @@ void CBinaryFileExternal::write(const char *name, IPropertyTree &tree)
 void CDeltaWriter::addToQueue(CTransactionItem *item)
 {
     bool needsSyncSave = false;
+    size_t items;
     {
         CriticalBlock b(pendingCrit);
         pending.push(item);
         // add actual size for externals, and nominal '100 byte' value for delta transactions
         // it will act. as a rough guide to appoaching size threshold. It is not worth
         // synchronously preparing and serializing here (which will be done asynchronously later)
         pendingSz += (CTransactionItem::f_addext == item->type) ? item->ext.dataLength : 100;
-        size_t items = pending.size();
+        items = pending.size();
         if ((pendingSz < transactionMaxMem) && (items < transactionQueueLimit))
         {
             if (lastSaveTime && ((get_cycles_now() - lastSaveTime) < thresholdDuration))
@@ -2293,12 +2288,12 @@ void CDeltaWriter::addToQueue(CTransactionItem *item)
         ++totalQueueLimitHits;
         // force a synchronous save
         CCycleTimer timer;
-        PROGLOG("Forcing synchronous save of %u transactions (pendingSz=%zu)", (unsigned)pending.size(), pendingSz);
+        PROGLOG("Forcing synchronous save of %u transactions (pendingSz=%zu)", (unsigned)items, pendingSz);
 
         // NB: ensure consistent lock ordering of blockedSaveCrit and pendingCrit
         CHECKEDCRITICALBLOCK(blockedSaveCrit, fakeCritTimeout); // because if Dali is saving state (::blockingSave), it will clear pending
         CriticalBlock b(pendingCrit);
-        size_t items = pending.size();
+        items = pending.size();
         // NB: items could be 0. It's possible that the delta writer has caught up and flushed pending already
         if (items && save(pending)) // if temporarily blocked, continue, meaning queue limit will overrun a bit (blocking window is short)
         {

esp/src/tests/v9-workunits.spec.ts

@@ -27,11 +27,8 @@ test.describe("V9 Workunits", () => {
         ];
 
         for (const column of expectedColumns) {
-            if (column === "Cluster") {
-                await expect(page.getByText(column, { exact: true })).toBeVisible();
-            } else {
-                await expect(page.getByText(column)).toBeVisible();
-            }
+            // Use column header selector with exact match to avoid ambiguity
+            await expect(page.getByRole("columnheader", { name: column, exact: true })).toBeVisible();
         }
 
         for (const column of costColumns) {
@@ -54,21 +51,25 @@ test.describe("V9 Workunits", () => {
         await page.getByRole("menuitem", { name: "Filter" }).click();
         await page.getByRole("textbox", { name: "Job Name" }).fill("global.setup.ts");
         await page.getByRole("button", { name: "Apply" }).click();
-        expect(await page.locator(".ms-DetailsRow")).not.toHaveCount(0);
+
+        // Wait for filter to apply and results to load
+        await page.waitForTimeout(1000);
+        await expect(page.locator(".ms-DetailsRow")).not.toHaveCount(0, { timeout: 10000 });
+
         await page.getByRole("menuitem", { name: "Filter" }).click();
     });
 
     test("Should allow protecting and unprotecting a workunit and reflect lock status", async ({ page, browserName }) => {
-        expect(await page.locator(".ms-DetailsRow")).not.toHaveCount(0);
+        await expect(page.locator(".ms-DetailsRow")).not.toHaveCount(0);
         await page.locator(".ms-DetailsRow").first().locator(".ms-DetailsRow-check").click();
-        expect(await page.locator(".ms-DetailsRow.is-selected")).toHaveCount(1);
+        await expect(page.locator(".ms-DetailsRow.is-selected")).toHaveCount(1);
         if (browserName === "chromium") {
             await page.getByRole("menuitem", { name: "Protect", exact: true }).click();
-            expect(await page.locator(".ms-DetailsRow").first().locator("[data-icon-name=\"LockSolid\"]")).toBeVisible();
+            await expect(page.locator(".ms-DetailsRow").first().locator("[data-icon-name=\"LockSolid\"]")).toBeVisible();
             await page.locator(".ms-DetailsRow").first().locator(".ms-DetailsRow-check").click();
-            expect(await page.locator(".ms-DetailsRow.is-selected")).toHaveCount(1);
+            await expect(page.locator(".ms-DetailsRow.is-selected")).toHaveCount(1);
             await page.getByRole("menuitem", { name: "Unprotect" }).click();
-            expect(await page.locator(".ms-DetailsRow").first().locator("[data-icon-name=\"LockSolid\"]")).toHaveCount(0);
+            await expect(page.locator(".ms-DetailsRow").first().locator("[data-icon-name=\"LockSolid\"]")).toHaveCount(0);
         }
     });
 
@@ -78,7 +79,10 @@ test.describe("V9 Workunits", () => {
         expect(await page.locator(".ms-DetailsRow.is-selected")).toHaveCount(1);
         if (browserName === "chromium") {
             await page.getByRole("menuitem", { name: "Set To Failed", exact: true }).click();
-            expect(await page.locator(".ms-DetailsRow").first()).toContainText("failed");
+
+            // Wait for the action to complete and UI to update
+            await page.waitForTimeout(1000);
+            await expect(page.locator(".ms-DetailsRow").first()).toContainText("failed", { timeout: 10000 });
         }
     });