From c7107c848b760e3e23eb21ea548487d7fb362606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Mon, 27 Jan 2025 18:32:47 +0100 Subject: [PATCH 1/4] feat(core): drop the closure table pls --- .../core/migrations/20250127172429_drop_closures.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 packages/server/modules/core/migrations/20250127172429_drop_closures.ts diff --git a/packages/server/modules/core/migrations/20250127172429_drop_closures.ts b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts new file mode 100644 index 0000000000..de0e17e07a --- /dev/null +++ b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts @@ -0,0 +1,9 @@ +import { Knex } from 'knex' + +export async function up(knex: Knex): Promise { + await knex.schema.dropTable('object_children_closure') +} + +export async function down(): Promise { + // do nothing, we do not care +} From 4f6f43b16b4814896c8ee7cb9c3a173429ed7d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Tue, 28 Jan 2025 08:47:04 +0100 Subject: [PATCH 2/4] feat(core): remove closures feature flag --- packages/server/modules/core/rest/upload.ts | 33 +++++-------------- packages/shared/src/environment/index.ts | 6 ---- .../speckle-server/templates/_helpers.tpl | 4 --- 3 files changed, 8 insertions(+), 35 deletions(-) diff --git a/packages/server/modules/core/rest/upload.ts b/packages/server/modules/core/rest/upload.ts index 288c37b065..62ef920099 100644 --- a/packages/server/modules/core/rest/upload.ts +++ b/packages/server/modules/core/rest/upload.ts @@ -1,29 +1,19 @@ import zlib from 'zlib' import { corsMiddleware } from '@/modules/core/configs/cors' import Busboy from 'busboy' -import { - getFeatureFlags, - maximumObjectUploadFileSizeMb -} from '@/modules/shared/helpers/envHelper' +import { maximumObjectUploadFileSizeMb } from '@/modules/shared/helpers/envHelper' import { ObjectHandlingError } from '@/modules/core/errors/object' import { estimateStringMegabyteSize } from '@/modules/core/utils/chunking' import { toMegabytesWith1DecimalPlace } from '@/modules/core/utils/formatting' import { Router } from 'express' -import { - createObjectsBatchedAndNoClosuresFactory, - createObjectsBatchedFactory -} from '@/modules/core/services/objects/management' -import { - storeClosuresIfNotFoundFactory, - storeObjectsIfNotFoundFactory -} from '@/modules/core/repositories/objects' +import { createObjectsBatchedAndNoClosuresFactory } from '@/modules/core/services/objects/management' +import { storeObjectsIfNotFoundFactory } from '@/modules/core/repositories/objects' import { validatePermissionsWriteStreamFactory } from '@/modules/core/services/streams/auth' import { authorizeResolver, validateScopes } from '@/modules/shared' import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' import { ExecuteHooks } from '@/modules/core/hooks' const MAX_FILE_SIZE = maximumObjectUploadFileSizeMb() * 1024 * 1024 -const { FF_NO_CLOSURE_WRITES } = getFeatureFlags() export default (app: Router, { executeHooks }: { executeHooks: ExecuteHooks }) => { const validatePermissionsWriteStream = validatePermissionsWriteStreamFactory({ @@ -70,18 +60,11 @@ export default (app: Router, { executeHooks }: { executeHooks: ExecuteHooks }) = const projectDb = await getProjectDbClient({ projectId: req.params.streamId }) - const objectInsertionService = FF_NO_CLOSURE_WRITES - ? createObjectsBatchedAndNoClosuresFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ - db: projectDb - }) - }) - : createObjectsBatchedFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ - db: projectDb - }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb }) - }) + const objectInsertionService = createObjectsBatchedAndNoClosuresFactory({ + storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ + db: projectDb + }) + }) let busboy try { diff --git a/packages/shared/src/environment/index.ts b/packages/shared/src/environment/index.ts index 4b2f1ee918..01cd6635cd 100644 --- a/packages/shared/src/environment/index.ts +++ b/packages/shared/src/environment/index.ts @@ -41,11 +41,6 @@ const parseFeatureFlags = () => { schema: z.boolean(), defaults: { production: false, _: true } }, - // Disables writing to the closure table in the create objects batched services (re object upload routes) - FF_NO_CLOSURE_WRITES: { - schema: z.boolean(), - defaults: { production: false, _: false } - }, // Enables workspaces multi region DB support FF_WORKSPACES_MULTI_REGION_ENABLED: { schema: z.boolean(), @@ -78,7 +73,6 @@ let parsedFlags: ReturnType | undefined export function getFeatureFlags(): { FF_AUTOMATE_MODULE_ENABLED: boolean FF_GENDOAI_MODULE_ENABLED: boolean - FF_NO_CLOSURE_WRITES: boolean FF_WORKSPACES_MODULE_ENABLED: boolean FF_WORKSPACES_SSO_ENABLED: boolean FF_GATEKEEPER_MODULE_ENABLED: boolean diff --git a/utils/helm/speckle-server/templates/_helpers.tpl b/utils/helm/speckle-server/templates/_helpers.tpl index 6e67a8217d..70d6bf4ccc 100644 --- a/utils/helm/speckle-server/templates/_helpers.tpl +++ b/utils/helm/speckle-server/templates/_helpers.tpl @@ -726,10 +726,6 @@ Generate the environment variables for Speckle server and Speckle objects deploy value: {{ .Values.server.asyncRequestContextEnabled | quote }} {{- end}} -# *** No more closures flag - prevents writing to the closure table *** -- name: FF_NO_CLOSURE_WRITES - value: {{ .Values.featureFlags.noClosureWrites | quote }} - # *** Gendo render module *** - name: FF_GENDOAI_MODULE_ENABLED value: {{ .Values.featureFlags.gendoAIModuleEnabled | quote }} From ba5ce8a5c6c22252cf617a067349f2fbccb0c554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Fri, 31 Jan 2025 11:14:12 +0100 Subject: [PATCH 3/4] feat(core): remove more closure stuff --- .../activitystream/tests/activity.spec.js | 6 +- .../modules/cli/commands/download/commit.ts | 4 +- .../modules/cli/commands/download/project.ts | 4 +- .../comments/tests/comments.graph.spec.js | 6 +- .../modules/comments/tests/comments.spec.ts | 6 +- .../modules/core/domain/objects/operations.ts | 9 +- .../modules/core/domain/objects/types.ts | 4 +- .../modules/core/graph/resolvers/objects.ts | 4 +- packages/server/modules/core/helpers/types.ts | 7 - .../20250127172429_drop_closures.ts | 21 ++- .../modules/core/repositories/objects.ts | 26 +--- .../core/services/objects/management.ts | 123 +----------------- .../modules/core/tests/branches.spec.ts | 6 +- .../server/modules/core/tests/commits.spec.ts | 6 +- .../server/modules/core/tests/objects.spec.js | 18 +-- .../server/modules/core/tests/streams.spec.ts | 4 +- .../server/modules/core/tests/users.spec.ts | 6 +- .../server/modules/cross-server-sync/index.ts | 4 +- .../server/modules/stats/tests/stats.spec.ts | 4 +- .../test/speckle-helpers/commitHelper.ts | 7 +- utils/helm/speckle-server/values.schema.json | 5 - utils/helm/speckle-server/values.yaml | 2 - 22 files changed, 56 insertions(+), 226 deletions(-) diff --git a/packages/server/modules/activitystream/tests/activity.spec.js b/packages/server/modules/activitystream/tests/activity.spec.js index 4d79869ccb..83c2bcb4dd 100644 --- a/packages/server/modules/activitystream/tests/activity.spec.js +++ b/packages/server/modules/activitystream/tests/activity.spec.js @@ -61,8 +61,7 @@ const { const { getServerInfoFactory } = require('@/modules/core/repositories/server') const { createObjectFactory } = require('@/modules/core/services/objects/management') const { - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } = require('@/modules/core/repositories/objects') const { getEventBus } = require('@/modules/shared/services/eventBus') @@ -121,8 +120,7 @@ const createPersonalAccessToken = createPersonalAccessTokenFactory({ storePersonalApiToken: storePersonalApiTokenFactory({ db }) }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) let server diff --git a/packages/server/modules/cli/commands/download/commit.ts b/packages/server/modules/cli/commands/download/commit.ts index 5e69332200..3b9c7d8ac4 100644 --- a/packages/server/modules/cli/commands/download/commit.ts +++ b/packages/server/modules/cli/commands/download/commit.ts @@ -16,7 +16,6 @@ import { import { getObjectFactory, getStreamObjectsFactory, - storeClosuresIfNotFoundFactory, storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { @@ -187,8 +186,7 @@ const command: CommandModule< const createObject = createObjectFactory({ storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db: projectDb - }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb }) + }) }) const getUser = getUserFactory({ db }) const getStreamCollaborators = getStreamCollaboratorsFactory({ db }) diff --git a/packages/server/modules/cli/commands/download/project.ts b/packages/server/modules/cli/commands/download/project.ts index 8789100a28..15dfb82ff7 100644 --- a/packages/server/modules/cli/commands/download/project.ts +++ b/packages/server/modules/cli/commands/download/project.ts @@ -19,7 +19,6 @@ import { createCommitByBranchIdFactory } from '@/modules/core/services/commit/ma import { getObjectFactory, getStreamObjectsFactory, - storeClosuresIfNotFoundFactory, storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { @@ -224,8 +223,7 @@ const command: CommandModule< const createObject = createObjectFactory({ storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db: projectDb - }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb }) + }) }) const getStreamCollaborators = getStreamCollaboratorsFactory({ db }) const getStreamBranchByName = getStreamBranchByNameFactory({ db: projectDb }) diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js index 6c2bc26291..40cb320b94 100644 --- a/packages/server/modules/comments/tests/comments.graph.spec.js +++ b/packages/server/modules/comments/tests/comments.graph.spec.js @@ -53,8 +53,7 @@ const { } = require('@/modules/core/repositories/streams') const { getObjectFactory, - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } = require('@/modules/core/repositories/objects') const { legacyCreateStreamFactory, @@ -219,8 +218,7 @@ const createUser = createUserFactory({ emitEvent: getEventBus().emit }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) function buildCommentInputFromString(textString) { diff --git a/packages/server/modules/comments/tests/comments.spec.ts b/packages/server/modules/comments/tests/comments.spec.ts index aa62f4dfe5..e9b77b1a60 100644 --- a/packages/server/modules/comments/tests/comments.spec.ts +++ b/packages/server/modules/comments/tests/comments.spec.ts @@ -69,8 +69,7 @@ import { } from '@/modules/core/repositories/branches' import { getObjectFactory, - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { legacyCreateStreamFactory, @@ -255,8 +254,7 @@ const createUser = createUserFactory({ emitEvent: getEventBus().emit }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) function buildCommentInputFromString(textString?: string) { diff --git a/packages/server/modules/core/domain/objects/operations.ts b/packages/server/modules/core/domain/objects/operations.ts index 7e9de7115e..ef13e8e7cb 100644 --- a/packages/server/modules/core/domain/objects/operations.ts +++ b/packages/server/modules/core/domain/objects/operations.ts @@ -2,8 +2,7 @@ import { Logger } from '@/logging/logging' import { InsertableSpeckleObject, RawSpeckleObject, - SpeckleObject, - SpeckleObjectClosureEntry + SpeckleObject } from '@/modules/core/domain/objects/types' import { BatchedSelectOptions } from '@/modules/shared/helpers/dbHelper' import { MaybeNullOrUndefined, Nullable, Optional } from '@speckle/shared' @@ -45,10 +44,6 @@ export type StoreObjectsIfNotFound = ( objects: Array ) => Promise -export type StoreClosuresIfNotFound = ( - closures: SpeckleObjectClosureEntry[] -) => Promise - export type GetObjectChildrenStream = (params: { streamId: string objectId: string @@ -117,8 +112,6 @@ type CreateObjectsParams = { logger?: Logger } -export type CreateObjectsBatched = (params: CreateObjectsParams) => Promise - export type CreateObjectsBatchedAndNoClosures = ( params: CreateObjectsParams ) => Promise diff --git a/packages/server/modules/core/domain/objects/types.ts b/packages/server/modules/core/domain/objects/types.ts index b960a19e2e..d06c75538d 100644 --- a/packages/server/modules/core/domain/objects/types.ts +++ b/packages/server/modules/core/domain/objects/types.ts @@ -1,9 +1,7 @@ -import { ObjectChildrenClosureRecord, ObjectRecord } from '@/modules/core/helpers/types' +import { ObjectRecord } from '@/modules/core/helpers/types' import { Nullable, NullableKeysToOptional } from '@speckle/shared' import { OverrideProperties, SetOptional } from 'type-fest' -export type SpeckleObjectClosureEntry = ObjectChildrenClosureRecord - export type SpeckleObject = ObjectRecord /** diff --git a/packages/server/modules/core/graph/resolvers/objects.ts b/packages/server/modules/core/graph/resolvers/objects.ts index 48ed2fdc07..66cef6e1a9 100644 --- a/packages/server/modules/core/graph/resolvers/objects.ts +++ b/packages/server/modules/core/graph/resolvers/objects.ts @@ -5,7 +5,6 @@ import { getObjectChildrenFactory, getObjectChildrenQueryFactory, getObjectFactory, - storeClosuresIfNotFoundFactory, storeObjectsIfNotFoundFactory } from '@/modules/core/repositories/objects' import { createObjectsFactory } from '@/modules/core/services/objects/management' @@ -109,8 +108,7 @@ export = { projectId: args.objectInput.streamId }) const createObjects = createObjectsFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db: projectDB }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDB }) + storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db: projectDB }) }) const ids = await createObjects({ streamId: args.objectInput.streamId, diff --git a/packages/server/modules/core/helpers/types.ts b/packages/server/modules/core/helpers/types.ts index 0dc8256cf6..9c19ddf4c3 100644 --- a/packages/server/modules/core/helpers/types.ts +++ b/packages/server/modules/core/helpers/types.ts @@ -136,13 +136,6 @@ export type ObjectRecord = { streamId: string } -export type ObjectChildrenClosureRecord = { - parent: string - child: string - minDepth: number - streamId: string -} - export type InvalidTokenResult = { valid: false } diff --git a/packages/server/modules/core/migrations/20250127172429_drop_closures.ts b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts index de0e17e07a..3432f6a299 100644 --- a/packages/server/modules/core/migrations/20250127172429_drop_closures.ts +++ b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts @@ -4,6 +4,23 @@ export async function up(knex: Knex): Promise { await knex.schema.dropTable('object_children_closure') } -export async function down(): Promise { - // do nothing, we do not care +export async function down(knex: Knex): Promise { + await knex.raw(` +-- Table Definition +CREATE TABLE "object_children_closure" ( + "parent" varchar(255) NOT NULL, + "child" varchar(255) NOT NULL, + "minDepth" int4 NOT NULL DEFAULT 1, + "streamId" varchar(10) NOT NULL, + CONSTRAINT "object_children_closure_streamid_foreign" FOREIGN KEY ("streamId") REFERENCES "public"."streams"("id") ON DELETE CASCADE +); +`) + + await knex.schema.alterTable('object_children_closure', (table) => { + table.index(['streamId', 'parent']) + table.index(['streamId', 'child']) + table.index(['streamId', 'minDepth']) + table.unique(['streamId', 'parent', 'child'], 'obj_parent_child_index') + table.index(['streamId', 'parent', 'minDepth'], 'full_pcd_index') + }) } diff --git a/packages/server/modules/core/repositories/objects.ts b/packages/server/modules/core/repositories/objects.ts index 851bf78fa3..709897f2ae 100644 --- a/packages/server/modules/core/repositories/objects.ts +++ b/packages/server/modules/core/repositories/objects.ts @@ -1,6 +1,6 @@ import { Optional } from '@speckle/shared' -import { buildTableHelper, knex, Objects } from '@/modules/core/dbSchema' -import { ObjectChildrenClosureRecord, ObjectRecord } from '@/modules/core/helpers/types' +import { knex, Objects } from '@/modules/core/dbSchema' +import { ObjectRecord } from '@/modules/core/helpers/types' import { BatchedSelectOptions, executeBatchedSelect @@ -16,7 +16,6 @@ import { GetObjectsStream, GetStreamObjects, HasObjects, - StoreClosuresIfNotFound, StoreObjects, StoreObjectsIfNotFound, StoreSingleObjectIfNotFound @@ -25,17 +24,8 @@ import { SpeckleObject } from '@/modules/core/domain/objects/types' import { SetOptional } from 'type-fest' import { get, set, toNumber } from 'lodash' -const ObjectChildrenClosure = buildTableHelper('object_children_closure', [ - 'parent', - 'child', - 'minDepth', - 'streamId' -]) - const tables = { - objects: (db: Knex) => db(Objects.name), - objectChildrenClosure: (db: Knex) => - db(ObjectChildrenClosure.name) + objects: (db: Knex) => db(Objects.name) } export const getStreamObjectsFactory = @@ -125,16 +115,6 @@ export const storeObjectsIfNotFoundFactory = .ignore() } -export const storeClosuresIfNotFoundFactory = - (deps: { db: Knex }): StoreClosuresIfNotFound => - async (closuresBatch) => { - await tables - .objectChildrenClosure(deps.db) - .insert(closuresBatch) - .onConflict() - .ignore() - } - export const getObjectChildrenStreamFactory = (deps: { db: Knex }): GetObjectChildrenStream => async ({ streamId, objectId }) => { diff --git a/packages/server/modules/core/services/objects/management.ts b/packages/server/modules/core/services/objects/management.ts index bbe10c7f31..2825f11b3a 100644 --- a/packages/server/modules/core/services/objects/management.ts +++ b/packages/server/modules/core/services/objects/management.ts @@ -1,8 +1,7 @@ import crypto from 'crypto' import { InsertableSpeckleObject, - RawSpeckleObject, - SpeckleObjectClosureEntry + RawSpeckleObject } from '@/modules/core/domain/objects/types' import { getMaximumObjectSizeMB } from '@/modules/shared/helpers/envHelper' import { @@ -14,13 +13,10 @@ import { servicesLogger } from '@/logging/logging' import { CreateObject, CreateObjects, - CreateObjectsBatched, CreateObjectsBatchedAndNoClosures, - StoreClosuresIfNotFound, StoreObjectsIfNotFound, StoreSingleObjectIfNotFound } from '@/modules/core/domain/objects/operations' -import { chunk } from 'lodash' /** * Note: we're generating the hash here, rather than on the db side, as there are @@ -59,23 +55,16 @@ const prepInsertionObject = ( export const createObjectFactory = (deps: { storeSingleObjectIfNotFoundFactory: StoreSingleObjectIfNotFound - storeClosuresIfNotFound: StoreClosuresIfNotFound }): CreateObject => async ({ streamId, object, logger = servicesLogger }) => { const insertionObject = prepInsertionObject(streamId, object) - const closures: Array = [] const totalChildrenCountByDepth: Record = {} + let totalChildrenCount = 0 if (object.__closure !== null) { for (const prop in object.__closure) { - closures.push({ - streamId, - parent: insertionObject.id, - child: prop, - minDepth: object.__closure[prop] - }) - + totalChildrenCount++ if (totalChildrenCountByDepth[object.__closure[prop].toString()]) totalChildrenCountByDepth[object.__closure[prop].toString()]++ else totalChildrenCountByDepth[object.__closure[prop].toString()] = 1 @@ -84,20 +73,12 @@ export const createObjectFactory = const finalInsertionObject: InsertableSpeckleObject = { ...insertionObject, - totalChildrenCount: closures.length, + totalChildrenCount, totalChildrenCountByDepth: JSON.stringify(totalChildrenCountByDepth) } await deps.storeSingleObjectIfNotFoundFactory(finalInsertionObject) - if (closures.length > 0) { - const batchSize = 10000 - while (closures.length > 0) { - const closuresBatch = closures.splice(0, batchSize) // splice so that we don't take up more memory - await deps.storeClosuresIfNotFound(closuresBatch) - } - } - logger.debug({ objectId: insertionObject.id }, 'Inserted object: {objectId}') return insertionObject.id @@ -108,84 +89,6 @@ const prepInsertionObjectBatch = (batch: InsertableSpeckleObject[]) => { batch.sort((a, b) => (a.id > b.id ? 1 : -1)) } -const prepInsertionClosureBatch = (batch: SpeckleObjectClosureEntry[]) => { - batch.sort((a, b) => - a.parent > b.parent ? 1 : a.parent === b.parent ? (a.child > b.child ? 1 : -1) : -1 - ) -} - -export const createObjectsBatchedFactory = - (deps: { - storeObjectsIfNotFoundFactory: StoreObjectsIfNotFound - storeClosuresIfNotFound: StoreClosuresIfNotFound - }): CreateObjectsBatched => - async ({ streamId, objects, logger = servicesLogger }) => { - const closures: SpeckleObjectClosureEntry[] = [] - const objsToInsert: InsertableSpeckleObject[] = [] - const ids: string[] = [] - - // Prep objects up - objects.forEach((obj) => { - const insertionObject = prepInsertionObject(streamId, obj) - let totalChildrenCountGlobal = 0 - const totalChildrenCountByDepth: Record = {} - - if (obj.__closure !== null) { - for (const prop in obj.__closure) { - closures.push({ - streamId, - parent: insertionObject.id, - child: prop, - minDepth: obj.__closure[prop] - }) - totalChildrenCountGlobal++ - if (totalChildrenCountByDepth[obj.__closure[prop].toString()]) - totalChildrenCountByDepth[obj.__closure[prop].toString()]++ - else totalChildrenCountByDepth[obj.__closure[prop].toString()] = 1 - } - } - - const finalInsertionObject: InsertableSpeckleObject = { - ...insertionObject, - totalChildrenCount: totalChildrenCountGlobal, - totalChildrenCountByDepth: JSON.stringify(totalChildrenCountByDepth) - } - - objsToInsert.push(finalInsertionObject) - ids.push(insertionObject.id) - }) - - const closureBatchSize = 1000 - const objectsBatchSize = 500 - - // step 1: insert objects - if (objsToInsert.length > 0) { - // const batches = chunk(objsToInsert, objectsBatchSize) - const batches = chunkInsertionObjectArray({ - objects: objsToInsert, - chunkLengthLimit: objectsBatchSize, - chunkSizeLimitMb: 2 - }) - for (const batch of batches) { - prepInsertionObjectBatch(batch) - await deps.storeObjectsIfNotFoundFactory(batch) - logger.info({ objectCount: batch.length }, 'Inserted {objectCount} objects') - } - } - - // step 2: insert closures - if (closures.length > 0) { - const batches = chunk(closures, closureBatchSize) - - for (const batch of batches) { - prepInsertionClosureBatch(batch) - await deps.storeClosuresIfNotFound(batch) - logger.info({ batchLength: batch.length }, 'Inserted {batchLength} closures') - } - } - return true - } - export const createObjectsBatchedAndNoClosuresFactory = (deps: { storeObjectsIfNotFoundFactory: StoreObjectsIfNotFound @@ -221,10 +124,7 @@ export const createObjectsBatchedAndNoClosuresFactory = } export const createObjectsFactory = - (deps: { - storeObjectsIfNotFoundFactory: StoreObjectsIfNotFound - storeClosuresIfNotFound: StoreClosuresIfNotFound - }): CreateObjects => + (deps: { storeObjectsIfNotFoundFactory: StoreObjectsIfNotFound }): CreateObjects => async ({ streamId, objects, logger = servicesLogger }) => { // TODO: Switch to knex batch inserting functionality // see http://knexjs.org/#Utility-BatchInsert @@ -242,7 +142,6 @@ export const createObjectsFactory = const ids: string[] = [] const insertBatch = async (batch: RawSpeckleObject[], index: number) => { - const closures: SpeckleObjectClosureEntry[] = [] const objsToInsert: InsertableSpeckleObject[] = [] const t0 = performance.now() @@ -255,13 +154,6 @@ export const createObjectsFactory = let totalChildrenCountGlobal = 0 if (obj.__closure !== null) { for (const prop in obj.__closure) { - closures.push({ - streamId, - parent: insertionObject.id, - child: prop, - minDepth: obj.__closure[prop] - }) - totalChildrenCountGlobal++ if (totalChildrenCountByDepth[obj.__closure[prop].toString()]) @@ -283,17 +175,12 @@ export const createObjectsFactory = await deps.storeObjectsIfNotFoundFactory(objsToInsert) } - if (closures.length > 0) { - await deps.storeClosuresIfNotFound(closures) - } - const t1 = performance.now() logger.info( { batchIndex: index + 1, totalCountOfBatches: batches.length, - countStoredObjects: closures.length + objsToInsert.length, elapsedTimeMs: t1 - t0 }, 'Batch {batchIndex}/{totalCountOfBatches}: Stored {countStoredObjects} objects in {elapsedTimeMs}ms.' diff --git a/packages/server/modules/core/tests/branches.spec.ts b/packages/server/modules/core/tests/branches.spec.ts index a0fc7e2235..e4e53922aa 100644 --- a/packages/server/modules/core/tests/branches.spec.ts +++ b/packages/server/modules/core/tests/branches.spec.ts @@ -44,8 +44,7 @@ import { } from '@/modules/core/repositories/commits' import { getObjectFactory, - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { legacyCreateStreamFactory, @@ -204,8 +203,7 @@ const getBranchesByStreamId = getPaginatedStreamBranchesFactory({ getStreamBranchCount: getStreamBranchCountFactory({ db }) }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) describe('Branches @core-branches', () => { diff --git a/packages/server/modules/core/tests/commits.spec.ts b/packages/server/modules/core/tests/commits.spec.ts index 5d22b2605c..3ff7b6313b 100644 --- a/packages/server/modules/core/tests/commits.spec.ts +++ b/packages/server/modules/core/tests/commits.spec.ts @@ -46,8 +46,7 @@ import { } from '@/modules/activitystream/services/commitActivity' import { getObjectFactory, - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { legacyCreateStreamFactory, @@ -227,8 +226,7 @@ const getCommitsByBranchName = getPaginatedBranchCommitsItemsByNameFactory({ getPaginatedBranchCommitsItems: getPaginatedBranchCommitsItemsFactory({ db }) }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) describe('Commits @core-commits', () => { diff --git a/packages/server/modules/core/tests/objects.spec.js b/packages/server/modules/core/tests/objects.spec.js index 9298fbb602..ce45c346ba 100644 --- a/packages/server/modules/core/tests/objects.spec.js +++ b/packages/server/modules/core/tests/objects.spec.js @@ -66,12 +66,11 @@ const { const { getServerInfoFactory } = require('@/modules/core/repositories/server') const { createObjectFactory, - createObjectsBatchedFactory, + createObjectsBatchedAndNoClosuresFactory, createObjectsFactory } = require('@/modules/core/services/objects/management') const { storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory, storeObjectsIfNotFoundFactory, getFormattedObjectFactory, getObjectChildrenStreamFactory, @@ -163,16 +162,13 @@ const createUser = createUserFactory({ emitEvent: getEventBus().emit }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) -const createObjectsBatched = createObjectsBatchedFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) +const createObjectsBatched = createObjectsBatchedAndNoClosuresFactory({ + storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }) }) const createObjects = createObjectsFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }) }) const getObject = getFormattedObjectFactory({ db }) const getObjectChildrenStream = getObjectChildrenStreamFactory({ db }) @@ -633,8 +629,8 @@ describe('Objects @core-objects', () => { await createObjectsBatched({ streamId: stream.id, objects: objs }) - const parent = await getObject({ streamId: stream.id, objectId: commitId }) - expect(parent.totalChildrenCount).to.equal(3333) + // const parent = await getObject({ streamId: stream.id, objectId: commitId }) + // expect(parent.totalChildrenCount).to.equal(3333) const commitChildren = await getObjectChildren({ streamId: stream.id, objectId: commitId, diff --git a/packages/server/modules/core/tests/streams.spec.ts b/packages/server/modules/core/tests/streams.spec.ts index 69f3588f4d..ec004d00a6 100644 --- a/packages/server/modules/core/tests/streams.spec.ts +++ b/packages/server/modules/core/tests/streams.spec.ts @@ -62,7 +62,6 @@ import { import { addCommitCreatedActivityFactory } from '@/modules/activitystream/services/commitActivity' import { getObjectFactory, - storeClosuresIfNotFoundFactory, storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { @@ -200,8 +199,7 @@ const isStreamCollaborator = isStreamCollaboratorFactory({ const grantPermissionsStream = grantStreamPermissionsFactory({ db }) const getStreamsUsers = getStreamsCollaboratorsFactory({ db }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) describe('Streams @core-streams', () => { diff --git a/packages/server/modules/core/tests/users.spec.ts b/packages/server/modules/core/tests/users.spec.ts index cdf894b205..3055a16c9e 100644 --- a/packages/server/modules/core/tests/users.spec.ts +++ b/packages/server/modules/core/tests/users.spec.ts @@ -41,8 +41,7 @@ import { } from '@/modules/core/repositories/streams' import { getObjectFactory, - storeSingleObjectIfNotFoundFactory, - storeClosuresIfNotFoundFactory + storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { legacyCreateStreamFactory, @@ -268,8 +267,7 @@ const getBranchesByStreamId = getPaginatedStreamBranchesFactory({ getStreamBranchCount: getStreamBranchCountFactory({ db }) }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) describe('Actors & Tokens @user-services', () => { diff --git a/packages/server/modules/cross-server-sync/index.ts b/packages/server/modules/cross-server-sync/index.ts index 1a53a8f9c0..3856c7bf2e 100644 --- a/packages/server/modules/cross-server-sync/index.ts +++ b/packages/server/modules/cross-server-sync/index.ts @@ -41,7 +41,6 @@ import { storeModelFactory } from '@/modules/core/repositories/models' import { getObjectFactory, getStreamObjectsFactory, - storeClosuresIfNotFoundFactory, storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { @@ -162,8 +161,7 @@ const crossServerSyncModule: SpeckleModule = { }) const createObject = createObjectFactory({ - storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }) }) const createNewProject = createNewProjectFactory({ diff --git a/packages/server/modules/stats/tests/stats.spec.ts b/packages/server/modules/stats/tests/stats.spec.ts index f0fd0c17b6..d06cf842a6 100644 --- a/packages/server/modules/stats/tests/stats.spec.ts +++ b/packages/server/modules/stats/tests/stats.spec.ts @@ -32,7 +32,6 @@ import { } from '@/modules/core/repositories/streams' import { getObjectFactory, - storeClosuresIfNotFoundFactory, storeObjectsIfNotFoundFactory } from '@/modules/core/repositories/objects' import { @@ -171,8 +170,7 @@ const createPersonalAccessToken = createPersonalAccessTokenFactory({ storePersonalApiToken: storePersonalApiTokenFactory({ db }) }) const createObjects = createObjectsFactory({ - storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db }) + storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db }) }) const params = { numUsers: 25, numStreams: 30, numObjects: 100, numCommits: 100 } diff --git a/packages/server/test/speckle-helpers/commitHelper.ts b/packages/server/test/speckle-helpers/commitHelper.ts index 3655623ac1..5f8afcaa58 100644 --- a/packages/server/test/speckle-helpers/commitHelper.ts +++ b/packages/server/test/speckle-helpers/commitHelper.ts @@ -13,7 +13,6 @@ import { } from '@/modules/core/repositories/commits' import { getObjectFactory, - storeClosuresIfNotFoundFactory, storeSingleObjectIfNotFoundFactory } from '@/modules/core/repositories/objects' import { markCommitStreamUpdatedFactory } from '@/modules/core/repositories/streams' @@ -65,8 +64,7 @@ export async function createTestObject(params: { projectId: string }) { const createObject = createObjectFactory({ storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db: projectDb - }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb }) + }) }) return await createObject({ @@ -86,8 +84,7 @@ async function ensureObjects(commits: BasicTestCommit[]) { const createObject = createObjectFactory({ storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db: projectDb - }), - storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb }) + }) }) return createObject({ diff --git a/utils/helm/speckle-server/values.schema.json b/utils/helm/speckle-server/values.schema.json index 500cefa3fa..5f0c665aae 100644 --- a/utils/helm/speckle-server/values.schema.json +++ b/utils/helm/speckle-server/values.schema.json @@ -45,11 +45,6 @@ "description": "High level flag that toggles the Gendo AI render module", "default": false }, - "noClosureWrites": { - "type": "boolean", - "description": "Toggles whether to stop writing to the closure table", - "default": false - }, "workspacesModuleEnabled": { "type": "boolean", "description": "High level flag fully toggles the workspaces module", diff --git a/utils/helm/speckle-server/values.yaml b/utils/helm/speckle-server/values.yaml index a7c227e68f..5f1d1daea9 100644 --- a/utils/helm/speckle-server/values.yaml +++ b/utils/helm/speckle-server/values.yaml @@ -39,8 +39,6 @@ featureFlags: automateModuleEnabled: false ## @param featureFlags.gendoAIModuleEnabled High level flag that toggles the Gendo AI render module gendoAIModuleEnabled: false - ## @param featureFlags.noClosureWrites Toggles whether to stop writing to the closure table - noClosureWrites: false ## @param featureFlags.workspacesModuleEnabled High level flag fully toggles the workspaces module workspacesModuleEnabled: false ## @param featureFlags.workspacesSSOEnabled High level flag fully toggles the workspaces dynamic sso From e6a1c52d4ee528fbaff3a4679b879823c5300a4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Fri, 7 Feb 2025 08:51:45 +0100 Subject: [PATCH 4/4] feat(core): only drop closure table if it exists --- .../modules/core/migrations/20250127172429_drop_closures.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/modules/core/migrations/20250127172429_drop_closures.ts b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts index 3432f6a299..87cc97dc40 100644 --- a/packages/server/modules/core/migrations/20250127172429_drop_closures.ts +++ b/packages/server/modules/core/migrations/20250127172429_drop_closures.ts @@ -1,7 +1,7 @@ import { Knex } from 'knex' export async function up(knex: Knex): Promise { - await knex.schema.dropTable('object_children_closure') + await knex.schema.dropTableIfExists('object_children_closure') } export async function down(knex: Knex): Promise {