From 02770e21ee8e1b5fe507936bcde0b22628100db9 Mon Sep 17 00:00:00 2001 From: nasc17 <69922333+nasc17@users.noreply.github.com> Date: Wed, 19 May 2021 13:46:24 -0700 Subject: [PATCH] Edit Postgres Engine Settings Per Role (#15481) * Enables being able to view and edit Coordinator node scheduling params (#15114) * Trying to save per role settings * Updated spec * Cleaning up * Removed unneccessary code and comments * Added separate type for { w?: string, c?: string}, PR fixes * Added methods to refresh mr,ml,cr,cl versus per role * Fixed spec * Put back optional properties, removed passing empty string to reset scheduling params * Spacing * vBump arc * Included roles in fake show output for testing (#15196) * Update arc specs (#15225) * Update azdata Arc specs to match April azdata * vcores -> cpu * fix spacing * Consolidate types and update storage volumes * Fix compile * Fix spec * include coordinator * Adding args * Query call success * Check for success in query * List full coordinator params * Change name * Update unit test for engine settings * Pr changes * Fix query Co-authored-by: Charles Gagnon --- extensions/arc/src/localizedConstants.ts | 3 +- extensions/arc/src/models/postgresModel.ts | 88 ++++++++++++++----- .../arc/src/test/mocks/fakeAzdataApi.ts | 2 + .../arc/src/test/models/postgresModel.test.ts | 13 ++- .../postgresCoordinatorNodeParametersPage.ts | 41 ++++----- .../dashboards/postgres/postgresDashboard.ts | 5 +- .../dashboards/postgres/postgresParameters.ts | 17 +++- .../postgresWorkerNodeParametersPage.ts | 15 ++-- extensions/azdata/src/api.ts | 2 + extensions/azdata/src/azdata.ts | 4 + extensions/azdata/src/typings/azdata-ext.d.ts | 8 +- 11 files changed, 131 insertions(+), 67 deletions(-) diff --git a/extensions/arc/src/localizedConstants.ts b/extensions/arc/src/localizedConstants.ts index a9d4078afb..3dcd9a606d 100644 --- a/extensions/arc/src/localizedConstants.ts +++ b/extensions/arc/src/localizedConstants.ts @@ -23,7 +23,6 @@ export const properties = localize('arc.properties', "Properties"); export const settings = localize('arc.settings', "Settings"); export const security = localize('arc.security', "Security"); export const computeAndStorage = localize('arc.computeAndStorage', "Compute + Storage"); -export const nodeParameters = localize('arc.nodeParameters', "Node Parameters"); export const coordinatorNodeParameters = localize('arc.coordinatorNodeParameters', "Coordinator Node Parameters"); export const workerNodeParameters = localize('arc.workerNodeParameters', "Worker Node Parameters"); export const compute = localize('arc.compute', "Compute"); @@ -146,7 +145,6 @@ export const databaseName = localize('arc.databaseName', "Database name"); export const enterNewPassword = localize('arc.enterNewPassword', "Enter a new password"); export const confirmNewPassword = localize('arc.confirmNewPassword', "Confirm the new password"); export const learnAboutPostgresClients = localize('arc.learnAboutPostgresClients', "Learn more about Azure PostgreSQL Hyperscale client interfaces"); -export const nodeParametersDescription = localize('arc.nodeParametersDescription', " These server parameters of the Coordinator node and the Worker nodes can be set to custom (non-default) values. Search to find parameters."); export const coordinatorNodeParametersDescription = localize('arc.coordinatorNodeParametersDescription', " These server parameters of the Coordinator node can be set to custom (non-default) values. Search to find parameters."); export const workerNodesParametersDescription = localize('arc.workerNodesParametersDescription', " These server parameters of the Worker nodes can be set to custom (non-default) values. Search to find parameters."); export const learnAboutNodeParameters = localize('arc.learnAboutNodeParameters', "Learn more about database engine settings for Azure Arc enabled PostgreSQL Hyperscale"); @@ -181,6 +179,7 @@ export const condition = localize('arc.condition', "Condition"); export const details = localize('arc.details', "Details"); export const lastTransition = localize('arc.lastTransition', "Last transition"); export const noExternalEndpoint = localize('arc.noExternalEndpoint', "No External Endpoint has been configured so this information isn't available."); +export const noWorkerPods = localize('arc.noWorkerPods', "No worker pods in this configuration."); export const podsReady = localize('arc.podsReady', "pods ready"); export const podsPresent = localize('arc.podsPresent', "Pods Present"); export const podsUsedDescription = localize('arc.podsUsedDescription', "Select a pod in the dropdown below for detailed health information."); diff --git a/extensions/arc/src/models/postgresModel.ts b/extensions/arc/src/models/postgresModel.ts index b4b2c3407b..28a38ba5be 100644 --- a/extensions/arc/src/models/postgresModel.ts +++ b/extensions/arc/src/models/postgresModel.ts @@ -155,36 +155,18 @@ export class PostgresModel extends ResourceModel { const provider = azdata.dataprotocol.getProvider(this._connectionProfile!.providerName, azdata.DataProviderType.QueryProvider); const ownerUri = await azdata.connection.getUriForConnection(this._activeConnectionId); - const engineSettings = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings'); - if (!engineSettings) { - throw new Error('Could not fetch engine settings'); - } - const skippedEngineSettings: String[] = [ 'archive_command', 'archive_timeout', 'log_directory', 'log_file_mode', 'log_filename', 'restore_command', 'shared_preload_libraries', 'synchronous_commit', 'ssl', 'unix_socket_permissions', 'wal_level' ]; - this.workerNodesEngineSettings = []; - - engineSettings.rows.forEach(row => { - let rowValues = row.map(c => c.displayValue); - let name = rowValues.shift(); - if (!skippedEngineSettings.includes(name!)) { - let result: EngineSettingsModel = { - parameterName: name, - value: rowValues.shift(), - description: rowValues.shift(), - min: rowValues.shift(), - max: rowValues.shift(), - options: rowValues.shift(), - type: rowValues.shift() - }; - - this.workerNodesEngineSettings.push(result); - } - }); + await this.createCoordinatorEngineSettings(provider, ownerUri, skippedEngineSettings); + const scale = this._config?.spec.scale; + const nodes = (scale?.workers ?? scale?.shards ?? 0); + if (nodes !== 0) { + await this.createWorkerEngineSettings(provider, ownerUri, skippedEngineSettings); + } this.engineSettingsLastUpdated = new Date(); this._engineSettingsPromise.resolve(); @@ -196,6 +178,64 @@ export class PostgresModel extends ResourceModel { } } + private async createCoordinatorEngineSettings(provider: azdata.QueryProvider, ownerUri: string, skip: String[]): Promise { + const engineSettingsCoordinator = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings'); + + this.coordinatorNodeEngineSettings = []; + engineSettingsCoordinator.rows.forEach(row => { + let rowValues = row.map(c => c.displayValue); + let name = rowValues.shift(); + if (!skip.includes(name!)) { + let result: EngineSettingsModel = { + parameterName: name, + value: rowValues.shift(), + description: rowValues.shift(), + min: rowValues.shift(), + max: rowValues.shift(), + options: rowValues.shift(), + type: rowValues.shift() + }; + + this.coordinatorNodeEngineSettings.push(result); + } + }); + + } + + private async createWorkerEngineSettings(provider: azdata.QueryProvider, ownerUri: string, skip: String[]): Promise { + + const engineSettingsWorker = await provider.runQueryAndReturn(ownerUri, + `with settings as (select nodename, success, result from run_command_on_workers('select json_agg(pg_settings) from pg_settings') order by success desc, nodename asc) + select * from settings limit case when exists(select 1 from settings where success) then 1 end`); + + if (engineSettingsWorker.rows[0][1].displayValue === 'False') { + let errorString = engineSettingsWorker.rows.map(row => row[2].displayValue); + throw new Error(errorString.join('\n')); + } + + let engineSettingsWorkerJSON = JSON.parse(engineSettingsWorker.rows[0][2].displayValue); + this.workerNodesEngineSettings = []; + + for (let i = 0; i < engineSettingsWorkerJSON.length; i++) { + let rowValues = engineSettingsWorkerJSON[i]; + let name = rowValues.name; + if (!skip.includes(name!)) { + let result: EngineSettingsModel = { + parameterName: name, + value: rowValues.setting, + description: rowValues.short_desc, + min: rowValues.min_val, + max: rowValues.max_val, + options: rowValues.enumvals, + type: rowValues.vartype + }; + + this.workerNodesEngineSettings.push(result); + } + } + + } + protected createConnectionProfile(): azdata.IConnectionProfile { const ipAndPort = parseIpAndPort(this.config?.status.primaryEndpoint || ''); return { diff --git a/extensions/arc/src/test/mocks/fakeAzdataApi.ts b/extensions/arc/src/test/mocks/fakeAzdataApi.ts index 3b7eebbdc1..023f86a9d0 100644 --- a/extensions/arc/src/test/mocks/fakeAzdataApi.ts +++ b/extensions/arc/src/test/mocks/fakeAzdataApi.ts @@ -33,6 +33,7 @@ export class FakeAzdataApi implements azdataExt.IAzdataApi { adminPassword?: boolean, coresLimit?: string, coresRequest?: string, + coordinatorEngineSettings?: string, engineSettings?: string, extensions?: string, memoryLimit?: string, @@ -40,6 +41,7 @@ export class FakeAzdataApi implements azdataExt.IAzdataApi { noWait?: boolean, port?: number, replaceEngineSettings?: boolean, + workerEngineSettings?: string, workers?: number }, _additionalEnvVars?: azdataExt.AdditionalEnvVars diff --git a/extensions/arc/src/test/models/postgresModel.test.ts b/extensions/arc/src/test/models/postgresModel.test.ts index 9d3054a8e2..8154576b1f 100644 --- a/extensions/arc/src/test/models/postgresModel.test.ts +++ b/extensions/arc/src/test/models/postgresModel.test.ts @@ -19,6 +19,11 @@ import { AzureArcTreeDataProvider } from '../../ui/tree/azureArcTreeDataProvider import { FakeControllerModel } from '../mocks/fakeControllerModel'; import { FakeAzdataApi } from '../mocks/fakeAzdataApi'; +export const FakeStorageVolume: azdataExt.StorageVolume[] = [{ + className: '', + size: '' +}]; + export const FakePostgresServerShowOutput: azdataExt.AzdataOutput = { logs: [], stdout: [], @@ -39,7 +44,11 @@ export const FakePostgresServerShowOutput: azdataExt.AzdataOutput { - /* TODO add correct azdata call for editing coordinator parameters - await this._azdataApi.azdata.arc.postgres.server.edit( - this._postgresModel.info.name, - { engineSettings: engineSettings.toString() }, - this._postgresModel.controllerModel.azdataAdditionalEnvVars, - session); - */ + protected async saveParameterEdits(engineSettings: string): Promise { + await this._azdataApi.azdata.arc.postgres.server.edit( + this._postgresModel.info.name, + { coordinatorEngineSettings: engineSettings }, + this._postgresModel.controllerModel.azdataAdditionalEnvVars, + this._postgresModel.controllerModel.controllerContext); + } protected async resetAllParameters(): Promise { - /* TODO add correct azdata call for editing coordinator parameters - await this._azdataApi.azdata.arc.postgres.server.edit( - this._postgresModel.info.name, - { engineSettings: `''`, replaceEngineSettings: true }, - this._postgresModel.controllerModel.azdataAdditionalEnvVars, - session); - */ + await this._azdataApi.azdata.arc.postgres.server.edit( + this._postgresModel.info.name, + { coordinatorEngineSettings: `''`, replaceEngineSettings: true }, + this._postgresModel.controllerModel.azdataAdditionalEnvVars, + this._postgresModel.controllerModel.controllerContext); } - protected async resetParameter(): Promise { - /* TODO add correct azdata call for editing coordinator parameters - await this._azdataApi.azdata.arc.postgres.server.edit( - this._postgresModel.info.name, - { engineSettings: parameterName + '=' }, - this._postgresModel.controllerModel.azdataAdditionalEnvVars, - session); - */ + protected async resetParameter(parameterName: string): Promise { + await this._azdataApi.azdata.arc.postgres.server.edit( + this._postgresModel.info.name, + { coordinatorEngineSettings: parameterName + '=' }, + this._postgresModel.controllerModel.azdataAdditionalEnvVars, + this._postgresModel.controllerModel.controllerContext); } } diff --git a/extensions/arc/src/ui/dashboards/postgres/postgresDashboard.ts b/extensions/arc/src/ui/dashboards/postgres/postgresDashboard.ts index 40d27034c1..2b487ebf20 100644 --- a/extensions/arc/src/ui/dashboards/postgres/postgresDashboard.ts +++ b/extensions/arc/src/ui/dashboards/postgres/postgresDashboard.ts @@ -17,6 +17,7 @@ import { PostgresComputeAndStoragePage } from './postgresComputeAndStoragePage'; import { PostgresWorkerNodeParametersPage } from './postgresWorkerNodeParametersPage'; import { PostgresPropertiesPage } from './postgresPropertiesPage'; import { PostgresResourceHealthPage } from './postgresResourceHealthPage'; +import { PostgresCoordinatorNodeParametersPage } from './postgresCoordinatorNodeParametersPage'; export class PostgresDashboard extends Dashboard { constructor(private _context: vscode.ExtensionContext, private _controllerModel: ControllerModel, private _postgresModel: PostgresModel) { @@ -36,8 +37,7 @@ export class PostgresDashboard extends Dashboard { const connectionStringsPage = new PostgresConnectionStringsPage(modelView, this.dashboard, this._postgresModel); const computeAndStoragePage = new PostgresComputeAndStoragePage(modelView, this.dashboard, this._postgresModel); const propertiesPage = new PostgresPropertiesPage(modelView, this.dashboard, this._controllerModel, this._postgresModel); - // TODO Add dashboard once backend is able to be connected for per role server parameter edits. - // const coordinatorNodeParametersPage = new PostgresCoordinatorNodeParametersPage(modelView, this._postgresModel); + const coordinatorNodeParametersPage = new PostgresCoordinatorNodeParametersPage(modelView, this.dashboard, this._postgresModel); const workerNodeParametersPage = new PostgresWorkerNodeParametersPage(modelView, this.dashboard, this._postgresModel); const diagnoseAndSolveProblemsPage = new PostgresDiagnoseAndSolveProblemsPage(modelView, this.dashboard, this._context, this._controllerModel, this._postgresModel); const supportRequestPage = new PostgresSupportRequestPage(modelView, this.dashboard, this._controllerModel, this._postgresModel); @@ -51,6 +51,7 @@ export class PostgresDashboard extends Dashboard { propertiesPage.tab, connectionStringsPage.tab, computeAndStoragePage.tab, + coordinatorNodeParametersPage.tab, workerNodeParametersPage.tab ] }, diff --git a/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts b/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts index 7b16926f30..50e4294a76 100644 --- a/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts +++ b/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts @@ -269,6 +269,13 @@ export abstract class PostgresParametersPage extends DashboardPage { this.disposables.push( this.connectToServerButton.onDidClick(async () => { + let scale = this._postgresModel.config?.spec.scale; + let nodes = (scale?.workers ?? scale?.shards ?? 0); + if (this.title === loc.workerNodeParameters && nodes === 0) { + vscode.window.showInformationMessage(loc.noWorkerPods); + return; + } + this.connectToServerButton!.enabled = false; if (!vscode.extensions.getExtension(loc.postgresExtension)) { const response = await vscode.window.showErrorMessage(loc.missingExtension('PostgreSQL'), loc.yes, loc.no); @@ -437,11 +444,13 @@ export abstract class PostgresParametersPage extends DashboardPage { let valueComponent: azdata.Component; if (engineSetting.type === 'enum') { // If type is enum, component should be drop down menu - let options = engineSetting.options?.slice(1, -1).split(','); let values: string[] = []; - options!.forEach(option => { - values.push(option.slice(option.indexOf('"') + 1, -1)); - }); + if (typeof engineSetting.options === 'string') { + let options = engineSetting.options?.slice(1, -1).split(','); + values = options.map(option => option.slice(option.indexOf('"') + 1, -1)); + } else if (engineSetting.options) { + values = engineSetting.options; + } let valueBox = this.modelView.modelBuilder.dropDown().withProps({ values: values, diff --git a/extensions/arc/src/ui/dashboards/postgres/postgresWorkerNodeParametersPage.ts b/extensions/arc/src/ui/dashboards/postgres/postgresWorkerNodeParametersPage.ts index 15a51ccfe5..38748dfae5 100644 --- a/extensions/arc/src/ui/dashboards/postgres/postgresWorkerNodeParametersPage.ts +++ b/extensions/arc/src/ui/dashboards/postgres/postgresWorkerNodeParametersPage.ts @@ -16,13 +16,11 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage { } protected get title(): string { - // TODO update to loc.workerNodeParameters - return loc.nodeParameters; + return loc.workerNodeParameters; } protected get id(): string { - // TODO update to 'postgres-worker-node-parameters' - return 'postgres-nodes-parameters'; + return 'postgres-worker-node-parameters'; } protected get icon(): { dark: string; light: string; } { @@ -30,8 +28,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage { } protected get description(): string { - // TODO update to loc.workerNodesParametersDescription - return loc.nodeParametersDescription; + return loc.workerNodesParametersDescription; } @@ -42,7 +39,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage { protected async saveParameterEdits(engineSettings: string): Promise { await this._azdataApi.azdata.arc.postgres.server.edit( this._postgresModel.info.name, - { engineSettings: engineSettings }, + { workerEngineSettings: engineSettings }, this._postgresModel.controllerModel.azdataAdditionalEnvVars, this._postgresModel.controllerModel.controllerContext); } @@ -50,7 +47,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage { protected async resetAllParameters(): Promise { await this._azdataApi.azdata.arc.postgres.server.edit( this._postgresModel.info.name, - { engineSettings: `''`, replaceEngineSettings: true }, + { workerEngineSettings: `''`, replaceEngineSettings: true }, this._postgresModel.controllerModel.azdataAdditionalEnvVars, this._postgresModel.controllerModel.controllerContext); } @@ -58,7 +55,7 @@ export class PostgresWorkerNodeParametersPage extends PostgresParametersPage { protected async resetParameter(parameterName: string): Promise { await this._azdataApi.azdata.arc.postgres.server.edit( this._postgresModel.info.name, - { engineSettings: parameterName + '=' }, + { workerEngineSettings: parameterName + '=' }, this._postgresModel.controllerModel.azdataAdditionalEnvVars, this._postgresModel.controllerModel.controllerContext); } diff --git a/extensions/azdata/src/api.ts b/extensions/azdata/src/api.ts index 4b35d8c7d4..23ba558c49 100644 --- a/extensions/azdata/src/api.ts +++ b/extensions/azdata/src/api.ts @@ -123,6 +123,7 @@ export function getAzdataApi(localAzdataDiscovered: Promise(argsArray, additionalEnvVars, azdataContext); } diff --git a/extensions/azdata/src/typings/azdata-ext.d.ts b/extensions/azdata/src/typings/azdata-ext.d.ts index 0cdfe04342..2cf5b61dfd 100644 --- a/extensions/azdata/src/typings/azdata-ext.d.ts +++ b/extensions/azdata/src/typings/azdata-ext.d.ts @@ -189,7 +189,11 @@ declare module 'azdata-ext' { name: string // "citus" }[], settings: { - default: { [key: string]: string } // { "max_connections": "101", "work_mem": "4MB" } + default: { [key: string]: string }, // { "max_connections": "101", "work_mem": "4MB" } + roles: { + coordinator: { [key: string]: string }, + worker: { [key: string]: string } + } }, version: string // "12" }, @@ -289,6 +293,7 @@ declare module 'azdata-ext' { adminPassword?: boolean, coresLimit?: string, coresRequest?: string, + coordinatorEngineSettings?: string, engineSettings?: string, extensions?: string, memoryLimit?: string, @@ -296,6 +301,7 @@ declare module 'azdata-ext' { noWait?: boolean, port?: number, replaceEngineSettings?: boolean, + workerEngineSettings?: string, workers?: number }, additionalEnvVars?: AdditionalEnvVars,