From 8b22d6b08bd145228d2bfee260721519c397e514 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 7 Mar 2024 09:33:02 +0000 Subject: [PATCH 1/5] Make user role type more representative of reality. --- packages/types/src/documents/global/userGroup.ts | 2 +- packages/types/src/sdk/events/userGroup.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/types/src/documents/global/userGroup.ts b/packages/types/src/documents/global/userGroup.ts index e6b70a160c..f914b2876d 100644 --- a/packages/types/src/documents/global/userGroup.ts +++ b/packages/types/src/documents/global/userGroup.ts @@ -24,7 +24,7 @@ export interface GroupUser { } export interface UserGroupRoles { - [key: string]: string + [key: string]: string | undefined } export interface SearchGroupRequest {} diff --git a/packages/types/src/sdk/events/userGroup.ts b/packages/types/src/sdk/events/userGroup.ts index ea1f554cff..804db5196d 100644 --- a/packages/types/src/sdk/events/userGroup.ts +++ b/packages/types/src/sdk/events/userGroup.ts @@ -48,7 +48,7 @@ export interface GroupAddedOnboardingEvent extends BaseEvent { } export interface GroupPermissionsEditedEvent extends BaseEvent { - permissions: Record + permissions: Record groupId: string audited: { name: string From 440dcb244deaca0d6cc34dd81b1d4bd0599dfbc3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 7 Mar 2024 11:11:36 +0100 Subject: [PATCH 2/5] Infinite retries --- packages/backend-core/src/cache/docWritethrough.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/cache/docWritethrough.ts b/packages/backend-core/src/cache/docWritethrough.ts index c8b28b3877..c69548adc4 100644 --- a/packages/backend-core/src/cache/docWritethrough.ts +++ b/packages/backend-core/src/cache/docWritethrough.ts @@ -2,20 +2,23 @@ import { AnyDocument, Database, LockName, LockType } from "@budibase/types" import { JobQueue, createQueue } from "../queue" import * as dbUtils from "../db" -import { string } from "yargs" -import { db } from ".." import { locks } from "../redis" import { Duration } from "../utils" interface ProcessDocMessage { dbName: string docId: string - data: Record } export const docWritethroughProcessorQueue = createQueue( - JobQueue.DOC_WRITETHROUGH_QUEUE + JobQueue.DOC_WRITETHROUGH_QUEUE, + { + jobOptions: { + // We might have plenty of 409, we want to allow running almost infinitely + attempts: Number.MAX_SAFE_INTEGER, + }, + } ) class DocWritethroughProcessor { From 17b06703e9c49fc2bebf9d63c26f817350750c20 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 7 Mar 2024 11:16:08 +0100 Subject: [PATCH 3/5] Remove lock --- .../backend-core/src/cache/docWritethrough.ts | 26 +++++++------------ packages/types/src/sdk/locks.ts | 1 - 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/backend-core/src/cache/docWritethrough.ts b/packages/backend-core/src/cache/docWritethrough.ts index c69548adc4..a47e62e155 100644 --- a/packages/backend-core/src/cache/docWritethrough.ts +++ b/packages/backend-core/src/cache/docWritethrough.ts @@ -1,9 +1,7 @@ -import { AnyDocument, Database, LockName, LockType } from "@budibase/types" +import { AnyDocument, Database } from "@budibase/types" import { JobQueue, createQueue } from "../queue" import * as dbUtils from "../db" -import { locks } from "../redis" -import { Duration } from "../utils" interface ProcessDocMessage { dbName: string @@ -24,21 +22,15 @@ export const docWritethroughProcessorQueue = createQueue( class DocWritethroughProcessor { init() { docWritethroughProcessorQueue.process(async message => { - const result = await locks.doWithLock( - { - type: LockType.TRY_ONCE, - name: LockName.PERSIST_DOC_WRITETHROUGH, - resource: `${message.data.dbName}:${message.data.docId}`, - ttl: Duration.fromSeconds(60).toMs(), - }, - async () => { - await this.persistToDb(message.data) + try { + await this.persistToDb(message.data) + } catch (err: any) { + if (err.status === 409) { + // If we get a 409, it means that another job updated it meanwhile. We want to retry it to persist it again. + throw new Error(`Conflict persisting message ${message.id}`) } - ) - if (!result.executed) { - throw new Error( - `Error persisting docWritethrough message: ${message.id}` - ) + + throw err } }) return this diff --git a/packages/types/src/sdk/locks.ts b/packages/types/src/sdk/locks.ts index 67de109657..c7c028a135 100644 --- a/packages/types/src/sdk/locks.ts +++ b/packages/types/src/sdk/locks.ts @@ -23,7 +23,6 @@ export enum LockName { APP_MIGRATION = "app_migrations", PROCESS_AUTO_COLUMNS = "process_auto_columns", PROCESS_USER_INVITE = "process_user_invite", - PERSIST_DOC_WRITETHROUGH = "persist_doc_writethrough", } export type LockOptions = { From 536422e60ba8a58ba6f223f32b1833beaa727c09 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 7 Mar 2024 11:23:09 +0100 Subject: [PATCH 4/5] Undo --- packages/backend-core/src/redis/redlockImpl.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 28babb9405..adeb5b12ec 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -82,11 +82,6 @@ type SuccessfulRedlockExecution = { } type UnsuccessfulRedlockExecution = { executed: false - reason: UnsuccessfulRedlockExecutionReason -} - -export const enum UnsuccessfulRedlockExecutionReason { - LockTakenWithTryOnce = "LOCK_TAKEN_WITH_TRY_ONCE", } type RedlockExecution = @@ -146,10 +141,7 @@ export async function doWithLock( if (opts.type === LockType.TRY_ONCE) { // don't throw for try-once locks, they will always error // due to retry count (0) exceeded - return { - executed: false, - reason: UnsuccessfulRedlockExecutionReason.LockTakenWithTryOnce, - } + return { executed: false } } else { throw e } From 9e55f87d0235b88f35e204476d4aac590f00bc8e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 7 Mar 2024 11:30:03 +0100 Subject: [PATCH 5/5] Add message --- packages/backend-core/src/cache/docWritethrough.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/backend-core/src/cache/docWritethrough.ts b/packages/backend-core/src/cache/docWritethrough.ts index a47e62e155..de119572f2 100644 --- a/packages/backend-core/src/cache/docWritethrough.ts +++ b/packages/backend-core/src/cache/docWritethrough.ts @@ -2,6 +2,7 @@ import { AnyDocument, Database } from "@budibase/types" import { JobQueue, createQueue } from "../queue" import * as dbUtils from "../db" +import { logWarn } from "../logging" interface ProcessDocMessage { dbName: string @@ -26,6 +27,7 @@ class DocWritethroughProcessor { await this.persistToDb(message.data) } catch (err: any) { if (err.status === 409) { + logWarn(`409 conflict in doc-writethrough cache`) // If we get a 409, it means that another job updated it meanwhile. We want to retry it to persist it again. throw new Error(`Conflict persisting message ${message.id}`) }