From 792e80b65d392d25a4cab28cf0bb90f39a9ff356 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Fri, 28 Apr 2023 10:23:16 -0400 Subject: [PATCH] [v9.5.x] SQL Datasources: Update behavior of default connection limits (#67465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * SQL Datasources: Update behavior of default connection limits (#66687) * Update behavior of defaults in connection limits * Refactor to use config object instead * Refactor remove unneeded function --------- Co-authored-by: Zoltán Bedi (cherry picked from commit 1fbac96bd495894f05956e97f5394bd0147f4a9b) * SQL Datasources: Update behavior of default connection limits (#66687) * Update behavior of defaults in connection limits * Refactor to use config object instead * Refactor remove unneeded function --------- Co-authored-by: Zoltán Bedi (cherry picked from commit 1fbac96bd495894f05956e97f5394bd0147f4a9b) --------- Co-authored-by: Kyle Cunningham Co-authored-by: Kyle Cunningham --- packages/grafana-data/src/types/config.ts | 7 ++++++ packages/grafana-runtime/src/config.ts | 5 +++++ pkg/api/dtos/frontend_settings.go | 8 +++++++ pkg/api/frontendsettings.go | 6 +++++ .../configuration/ConnectionLimits.tsx | 12 +++++----- .../useMigrateDatabaseFields.test.ts | 19 +++++++++++++--- .../configuration/useMigrateDatabaseFields.ts | 22 ++++++++++++++----- public/app/features/plugins/sql/constants.ts | 8 ------- 8 files changed, 65 insertions(+), 22 deletions(-) diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index 07c8186f92d..1db14beb302 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -223,6 +223,13 @@ export interface GrafanaConfig { rudderstackDataPlaneUrl: string | undefined; rudderstackSdkUrl: string | undefined; rudderstackConfigUrl: string | undefined; + sqlConnectionLimits: SqlConnectionLimits; +} + +export interface SqlConnectionLimits { + maxOpenConns: number; + maxIdleConns: number; + connMaxLifetime: number; } export interface AuthSettings { diff --git a/packages/grafana-runtime/src/config.ts b/packages/grafana-runtime/src/config.ts index 64b0450317d..51bc8d33dbb 100644 --- a/packages/grafana-runtime/src/config.ts +++ b/packages/grafana-runtime/src/config.ts @@ -151,6 +151,11 @@ export class GrafanaBootConfig implements GrafanaConfig { rudderstackDataPlaneUrl: undefined; rudderstackSdkUrl: undefined; rudderstackConfigUrl: undefined; + sqlConnectionLimits = { + maxOpenConns: 100, + maxIdleConns: 100, + connMaxLifetime: 14400, + }; tokenExpirationDayLimit: undefined; diff --git a/pkg/api/dtos/frontend_settings.go b/pkg/api/dtos/frontend_settings.go index c956ae87069..fdbf8977719 100644 --- a/pkg/api/dtos/frontend_settings.go +++ b/pkg/api/dtos/frontend_settings.go @@ -121,6 +121,12 @@ type FrontendSettingsWhitelabelingDTO struct { PublicDashboardFooter *FrontendSettingsPublicDashboardFooterConfigDTO `json:"publicDashboardFooter,omitempty"` // PR TODO: type this properly } +type FrontendSettingsSqlConnectionLimitsDTO struct { + MaxOpenConns int `json:"maxOpenConns"` + MaxIdleConns int `json:"maxIdleConns"` + ConnMaxLifetime int `json:"connMaxLifetime"` +} + type FrontendSettingsDTO struct { DefaultDatasource string `json:"defaultDatasource"` Datasources map[string]plugins.DataSourceDTO `json:"datasources"` @@ -223,6 +229,8 @@ type FrontendSettingsDTO struct { PluginsCDNBaseURL string `json:"pluginsCDNBaseURL,omitempty"` + SqlConnectionLimits FrontendSettingsSqlConnectionLimitsDTO `json:"sqlConnectionLimits"` + // Enterprise Licensing *FrontendSettingsLicensingDTO `json:"licensing,omitempty"` Whitelabeling *FrontendSettingsWhitelabelingDTO `json:"whitelabeling,omitempty"` diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index fbd626e9c80..7293b1bc458 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -215,6 +215,12 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro TokenExpirationDayLimit: hs.Cfg.SATokenExpirationDayLimit, SnapshotEnabled: hs.Cfg.SnapshotEnabled, + + SqlConnectionLimits: dtos.FrontendSettingsSqlConnectionLimitsDTO{ + MaxOpenConns: hs.Cfg.SqlDatasourceMaxOpenConnsDefault, + MaxIdleConns: hs.Cfg.SqlDatasourceMaxIdleConnsDefault, + ConnMaxLifetime: hs.Cfg.SqlDatasourceMaxConnLifetimeDefault, + }, } if hs.Cfg.UnifiedAlerting.Enabled != nil { diff --git a/public/app/features/plugins/sql/components/configuration/ConnectionLimits.tsx b/public/app/features/plugins/sql/components/configuration/ConnectionLimits.tsx index aa732c1ad27..d7f97b8620b 100644 --- a/public/app/features/plugins/sql/components/configuration/ConnectionLimits.tsx +++ b/public/app/features/plugins/sql/components/configuration/ConnectionLimits.tsx @@ -1,10 +1,10 @@ import React from 'react'; import { DataSourceSettings } from '@grafana/data'; +import { config } from '@grafana/runtime'; import { FieldSet, InlineField, InlineFieldRow, InlineSwitch } from '@grafana/ui'; import { NumberInput } from 'app/core/components/OptionsUI/NumberInput'; -import { SQLConnectionDefaults } from '../../constants'; import { SQLConnectionLimits, SQLOptions } from '../../types'; interface Props { @@ -47,7 +47,7 @@ export const ConnectionLimits = (props: Props) maxOpenConns: number, maxIdleConns: number, }); - } else if (number !== undefined) { + } else { updateJsonData({ maxOpenConns: number, }); @@ -68,10 +68,10 @@ export const ConnectionLimits = (props: Props) if (jsonData.maxOpenConns !== undefined) { maxConns = jsonData.maxOpenConns; idleConns = jsonData.maxOpenConns; - } else { - maxConns = SQLConnectionDefaults.MAX_CONNS; - idleConns = SQLConnectionDefaults.MAX_CONNS; } + } else { + maxConns = jsonData.maxOpenConns; + idleConns = jsonData.maxIdleConns; } updateJsonData({ @@ -124,7 +124,7 @@ export const ConnectionLimits = (props: Props) If enabled, automatically set the number of Maximum idle connections to the same value as Max open connections. If the number of maximum open connections is not set it will be set to the - default ({SQLConnectionDefaults.MAX_CONNS}). + default ({config.sqlConnectionLimits.maxIdleConns}). } > diff --git a/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.test.ts b/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.test.ts index a687b85c304..cda44ce5924 100644 --- a/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.test.ts +++ b/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.test.ts @@ -2,11 +2,23 @@ import { renderHook } from '@testing-library/react-hooks'; import { DataSourceSettings } from '@grafana/data'; -import { SQLConnectionDefaults } from '../../constants'; import { SQLOptions } from '../../types'; import { useMigrateDatabaseFields } from './useMigrateDatabaseFields'; +jest.mock('@grafana/runtime', () => { + return { + config: { + sqlConnectionLimits: { + maxOpenConns: 10, + maxIdleConns: 11, + connMaxLifetime: 12, + }, + }, + logDebug: jest.fn(), + }; +}); + describe('Database Field Migration', () => { let defaultProps = { options: { @@ -57,8 +69,9 @@ describe('Database Field Migration', () => { ...defaultProps, onOptionsChange: (options: DataSourceSettings) => { const jsonData = options.jsonData as SQLOptions; - expect(jsonData.maxOpenConns).toBe(SQLConnectionDefaults.MAX_CONNS); - expect(jsonData.maxIdleConns).toBe(Math.ceil(SQLConnectionDefaults.MAX_CONNS)); + expect(jsonData.maxOpenConns).toBe(10); + expect(jsonData.maxIdleConns).toBe(11); + expect(jsonData.connMaxLifetime).toBe(12); expect(jsonData.maxIdleConnsAuto).toBe(true); }, }; diff --git a/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.ts b/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.ts index cf587f1b938..2a7ffbe8458 100644 --- a/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.ts +++ b/public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.ts @@ -1,9 +1,8 @@ import { useEffect } from 'react'; import { DataSourcePluginOptionsEditorProps } from '@grafana/data'; -import { logDebug } from '@grafana/runtime'; +import { logDebug, config } from '@grafana/runtime'; -import { SQLConnectionDefaults } from '../../constants'; import { SQLOptions } from '../../types'; /** @@ -34,9 +33,7 @@ export function useMigrateDatabaseFields({ jsonData.maxIdleConns === undefined && jsonData.maxIdleConnsAuto === undefined ) { - // It's expected that the default will be greater than 4 - const maxOpenConns = SQLConnectionDefaults.MAX_CONNS; - const maxIdleConns = maxOpenConns; + const { maxOpenConns, maxIdleConns } = config.sqlConnectionLimits; logDebug( `Setting default max open connections to ${maxOpenConns} and setting max idle connection to ${maxIdleConns}` @@ -55,6 +52,21 @@ export function useMigrateDatabaseFields({ optionsUpdated = true; } + // If the maximum connection lifetime hasn't been + // otherwise set fill in with the default from configuration + if (jsonData.connMaxLifetime === undefined) { + const { connMaxLifetime } = config.sqlConnectionLimits; + + // Spread new options and add our value + newOptions.jsonData = { + ...newOptions.jsonData, + connMaxLifetime: connMaxLifetime, + }; + + // Note that we've updated the options + optionsUpdated = true; + } + // Only issue an update if we changed options if (optionsUpdated) { onOptionsChange(newOptions); diff --git a/public/app/features/plugins/sql/constants.ts b/public/app/features/plugins/sql/constants.ts index af6a4758786..0d3466aecc6 100644 --- a/public/app/features/plugins/sql/constants.ts +++ b/public/app/features/plugins/sql/constants.ts @@ -15,11 +15,3 @@ export const MACRO_NAMES = [ '$__unixEpochGroup', '$__unixEpochGroupAlias', ]; - -/** - * Constants for SQL connection - * parameters and automatic settings - */ -export const SQLConnectionDefaults = { - MAX_CONNS: 100, -};