From 9dfa8c76514a2f6b7e224e2546803a9fab96d117 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 22 Nov 2021 17:42:41 +0000 Subject: [PATCH] Fixing issue from review, values weren't wiped when delete modals closed, also adding a fix for threading to disable it in development as node-ts was causing memory leaks on low memory systems (doesn't apply to production built version). --- .../DataTable/modals/CreateEditColumn.svelte | 1 + .../popovers/EditTablePopover.svelte | 5 +++++ packages/server/scripts/dev/manage.js | 1 + packages/server/src/environment.js | 15 +++++++++++++++ packages/server/src/threads/automation.js | 4 +++- packages/server/src/threads/index.js | 12 +++++++++--- packages/server/src/threads/query.js | 3 +++ 7 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 86ad625c2c..841e781cf2 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -180,6 +180,7 @@ function hideDeleteDialog() { confirmDeleteDialog.hide() + deleteColName = "" deletion = false } diff --git a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte index 0c0f8efe4f..6d8c5811ec 100644 --- a/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte +++ b/packages/builder/src/components/backend/TableNavigator/popovers/EditTablePopover.svelte @@ -54,6 +54,10 @@ } } + function hideDeleteDialog() { + deleteTableName = "" + } + async function save() { await tables.save(table) notifications.success("Table renamed successfully") @@ -98,6 +102,7 @@ bind:this={confirmDeleteDialog} okText="Delete Table" onOk={deleteTable} + onCancel={hideDeleteDialog} title="Confirm Deletion" disabled={deleteTableName !== table.name} > diff --git a/packages/server/scripts/dev/manage.js b/packages/server/scripts/dev/manage.js index bd91056f84..f3938ad8ee 100644 --- a/packages/server/scripts/dev/manage.js +++ b/packages/server/scripts/dev/manage.js @@ -50,6 +50,7 @@ async function init() { SELF_HOSTED: 1, DISABLE_ACCOUNT_PORTAL: "", MULTI_TENANCY: "", + DISABLE_THREADING: 1, } let envFile = "" Object.keys(envFileJson).forEach(key => { diff --git a/packages/server/src/environment.js b/packages/server/src/environment.js index 4044bb3787..925fede086 100644 --- a/packages/server/src/environment.js +++ b/packages/server/src/environment.js @@ -23,6 +23,8 @@ if (!LOADED && isDev() && !isTest()) { LOADED = true } +let inThread = false + module.exports = { // important PORT: process.env.PORT, @@ -62,6 +64,7 @@ module.exports = { USERID_API_KEY: process.env.USERID_API_KEY, DEPLOYMENT_CREDENTIALS_URL: process.env.DEPLOYMENT_CREDENTIALS_URL, ALLOW_DEV_AUTOMATIONS: process.env.ALLOW_DEV_AUTOMATIONS, + DISABLE_THREADING: process.env.DISABLE_THREADING, _set(key, value) { process.env[key] = value module.exports[key] = value @@ -72,6 +75,18 @@ module.exports = { isProd: () => { return !isDev() }, + // used to check if already in a thread, don't thread further + setInThread: () => { + inThread = true + }, + isInThread: () => { + return inThread + }, +} + +// threading can cause memory issues with node-ts in development +if (isDev() && module.exports.DISABLE_THREADING == null) { + module.exports._set("DISABLE_THREADING", "1") } // clean up any environment variable edge cases diff --git a/packages/server/src/threads/automation.js b/packages/server/src/threads/automation.js index 1cd751e895..11ee28dbe8 100644 --- a/packages/server/src/threads/automation.js +++ b/packages/server/src/threads/automation.js @@ -1,3 +1,6 @@ +// when thread starts, make sure it is recorded +const env = require("../environment") +env.setInThread() const actions = require("../automations/actions") const automationUtils = require("../automations/automationUtils") const AutomationEmitter = require("../events/AutomationEmitter") @@ -6,7 +9,6 @@ const { DEFAULT_TENANT_ID } = require("@budibase/auth").constants const CouchDB = require("../db") const { DocumentTypes, isDevAppID } = require("../db/utils") const { doInTenant } = require("@budibase/auth/tenancy") -const env = require("../environment") const usage = require("../utilities/usageQuota") const { definitions: triggerDefs } = require("../automations/triggerInfo") diff --git a/packages/server/src/threads/index.js b/packages/server/src/threads/index.js index e24b45bbfa..94571c31d1 100644 --- a/packages/server/src/threads/index.js +++ b/packages/server/src/threads/index.js @@ -24,10 +24,16 @@ function typeToFile(type) { class Thread { constructor(type, opts = { timeoutMs: null, count: 1 }) { this.type = type - if (!env.isTest()) { + this.count = opts.count ? opts.count : 1 + this.disableThreading = + env.isTest() || + env.DISABLE_THREADING || + this.count === 0 || + env.isInThread() + if (!this.disableThreading) { const workerOpts = { autoStart: true, - maxConcurrentWorkers: opts.count ? opts.count : 1, + maxConcurrentWorkers: this.count, } if (opts.timeoutMs) { workerOpts.maxCallTime = opts.timeoutMs @@ -40,7 +46,7 @@ class Thread { return new Promise((resolve, reject) => { let fncToCall // if in test then don't use threading - if (env.isTest()) { + if (this.disableThreading) { fncToCall = require(typeToFile(this.type)) } else { fncToCall = this.workers diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js index ca8bf2161c..6b0d021e94 100644 --- a/packages/server/src/threads/query.js +++ b/packages/server/src/threads/query.js @@ -1,3 +1,6 @@ +// when thread starts, make sure it is recorded +const env = require("../environment") +env.setInThread() const ScriptRunner = require("../utilities/scriptRunner") const { integrations } = require("../integrations")