Merge pull request #13606 from Budibase/test-race-condition

Make auto ID row creation in parallel more robust.

packages/server/src/api/controllers/row/staticFormula.ts

@@ -1,20 +1,11 @@
 import { getRowParams } from "../../../db/utils"
 import {
   outputProcessing,
-  processAutoColumn,
   processFormulas,
 } from "../../../utilities/rowProcessor"
-import { context, locks } from "@budibase/backend-core"
+import { context } from "@budibase/backend-core"
-import {
+import { Table, Row, FormulaType, FieldType } from "@budibase/types"
-  Table,
-  Row,
-  LockType,
-  LockName,
-  FormulaType,
-  FieldType,
-} from "@budibase/types"
 import * as linkRows from "../../../db/linkedRows"
-import sdk from "../../../sdk"
 import isEqual from "lodash/isEqual"
 import { cloneDeep } from "lodash/fp"
 
@@ -151,30 +142,7 @@ export async function finaliseRow(
   // if another row has been written since processing this will
   // handle the auto ID clash
   if (oldTable && !isEqual(oldTable, table)) {
-    try {
     await db.put(table)
-    } catch (err: any) {
-      if (err.status === 409) {
-        // Some conflicts with the autocolumns occurred, we need to refetch the table and recalculate
-        await locks.doWithLock(
-          {
-            type: LockType.AUTO_EXTEND,
-            name: LockName.PROCESS_AUTO_COLUMNS,
-            resource: table._id,
-          },
-          async () => {
-            const latestTable = await sdk.tables.getTable(table._id!)
-            let response = processAutoColumn(null, latestTable, row, {
-              reprocessing: true,
-            })
-            await db.put(response.table)
-            row = response.row
-          }
-        )
-      } else {
-        throw err
-      }
-    }
   }
   const response = await db.put(row)
   // for response, calculate the formulas for the enriched row

packages/server/src/sdk/app/rows/tests/internal.spec.ts

@@ -145,6 +145,7 @@ describe("sdk >> rows >> internal", () => {
                 lastID: 1,
               },
             },
+            _rev: expect.stringMatching("2-.*"),
           },
           row: {
             ...row,
@@ -189,7 +190,6 @@ describe("sdk >> rows >> internal", () => {
             type: FieldType.AUTO,
             subtype: AutoFieldSubType.AUTO_ID,
             autocolumn: true,
-            lastID: 0,
           },
         },
       })
@@ -199,7 +199,7 @@ describe("sdk >> rows >> internal", () => {
           await internalSdk.save(table._id!, row, config.getUser()._id)
         }
         await Promise.all(
-          makeRows(10).map(row =>
+          makeRows(20).map(row =>
             internalSdk.save(table._id!, row, config.getUser()._id)
           )
         )
@@ -209,19 +209,21 @@ describe("sdk >> rows >> internal", () => {
       })
 
       const persistedRows = await config.getRows(table._id!)
-      expect(persistedRows).toHaveLength(20)
+      expect(persistedRows).toHaveLength(30)
       expect(persistedRows).toEqual(
         expect.arrayContaining(
-          Array.from({ length: 20 }).map((_, i) =>
+          Array.from({ length: 30 }).map((_, i) =>
             expect.objectContaining({ id: i + 1 })
           )
         )
       )
 
       const persistedTable = await config.getTable(table._id)
-      expect((table.schema.id as AutoColumnFieldMetadata).lastID).toBe(0)
+      expect(
+        (table.schema.id as AutoColumnFieldMetadata).lastID
+      ).toBeUndefined()
       expect((persistedTable.schema.id as AutoColumnFieldMetadata).lastID).toBe(
-        20
+        30
       )
     })
   })

packages/server/src/utilities/rowProcessor/index.ts

@@ -1,6 +1,6 @@
 import * as linkRows from "../../db/linkedRows"
 import { processFormulas, fixAutoColumnSubType } from "./utils"
-import { objectStore, utils } from "@budibase/backend-core"
+import { context, objectStore, utils } from "@budibase/backend-core"
 import { InternalTables } from "../../db/utils"
 import { TYPE_TRANSFORM_MAP } from "./map"
 import {
@@ -25,7 +25,44 @@ type AutoColumnProcessingOpts = {
   noAutoRelationships?: boolean
 }
 
-const BASE_AUTO_ID = 1
+// Returns the next auto ID for a column in a table. On success, the table will
+// be updated which is why it gets returned. The nextID returned is guaranteed
+// to be given only to you, and if you don't use it it's gone forever (a gap
+// will be left in the auto ID sequence).
+//
+// This function can throw if it fails to generate an auto ID after so many
+// attempts.
+async function getNextAutoId(
+  table: Table,
+  column: string
+): Promise<{ table: Table; nextID: number }> {
+  const db = context.getAppDB()
+  for (let attempt = 0; attempt < 5; attempt++) {
+    const schema = table.schema[column]
+    if (schema.type !== FieldType.NUMBER && schema.type !== FieldType.AUTO) {
+      throw new Error(`Column ${column} is not an auto column`)
+    }
+    schema.lastID = (schema.lastID || 0) + 1
+    try {
+      const resp = await db.put(table)
+      table._rev = resp.rev
+      return { table, nextID: schema.lastID }
+    } catch (e: any) {
+      if (e.status !== 409) {
+        throw e
+      }
+      // We wait for a random amount of time before retrying. The randomness
+      // makes it less likely for multiple requests modifying this table to
+      // collide.
+      await new Promise(resolve =>
+        setTimeout(resolve, Math.random() * 1.2 ** attempt * 1000)
+      )
+      table = await db.get(table._id)
+    }
+  }
+
+  throw new Error("Failed to generate an auto ID")
+}
 
 /**
  * This will update any auto columns that are found on the row/table with the correct information based on
@@ -37,7 +74,7 @@ const BASE_AUTO_ID = 1
  * @returns The updated row and table, the table may need to be updated
  * for automatic ID purposes.
  */
-export function processAutoColumn(
+export async function processAutoColumn(
   userId: string | null | undefined,
   table: Table,
   row: Row,
@@ -79,8 +116,9 @@ export function processAutoColumn(
         break
       case AutoFieldSubType.AUTO_ID:
         if (creating) {
-          schema.lastID = !schema.lastID ? BASE_AUTO_ID : schema.lastID + 1
+          const { table: newTable, nextID } = await getNextAutoId(table, key)
-          row[key] = schema.lastID
+          table = newTable
+          row[key] = nextID
         }
         break
     }

packages/types/src/sdk/locks.ts

@@ -21,7 +21,6 @@ export enum LockName {
   PERSIST_WRITETHROUGH = "persist_writethrough",
   QUOTA_USAGE_EVENT = "quota_usage_event",
   APP_MIGRATION = "app_migrations",
-  PROCESS_AUTO_COLUMNS = "process_auto_columns",
   PROCESS_USER_INVITE = "process_user_invite",
 }