From a8eed6114b9b9b36fe2cd1902b690090bc547ca4 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Sat, 2 Nov 2019 14:58:56 -0700 Subject: [PATCH] Fix race conditions with BDC dashboard init (#8135) * Fix race conditions with initialized * Check for undefined status * Disable no-floating-promises check * Add catch instead of disabling tslint disable --- .../src/bigDataCluster/dialog/bdcDashboard.ts | 19 ++-- .../dialog/bdcDashboardOverviewPage.ts | 23 ++--- .../bigDataCluster/dialog/bdcDashboardPage.ts | 40 +++++++++ .../dialog/bdcDashboardResourceStatusPage.ts | 16 ++-- .../dialog/bdcServiceStatusPage.ts | 90 +++++++++---------- 5 files changed, 109 insertions(+), 79 deletions(-) create mode 100644 extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardPage.ts diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts index 9e696f66b35d..dfd8686636ac 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts @@ -3,8 +3,6 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -'use strict'; - import * as azdata from 'azdata'; import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; @@ -15,6 +13,7 @@ import { BdcDashboardOverviewPage } from './bdcDashboardOverviewPage'; import { BdcStatusModel, ServiceStatusModel } from '../controller/apiGenerated'; import { getHealthStatusDot, getServiceNameDisplayText, showErrorMessage } from '../utils'; import { HdfsDialogCancelledError } from './hdfsDialogBase'; +import { BdcDashboardPage } from './bdcDashboardPage'; const localize = nls.loadMessageBundle(); @@ -25,12 +24,10 @@ const unselectedTabCss = { 'font-weight': '' }; type NavTab = { serviceName: string, div: azdata.DivContainer, dot: azdata.TextComponent, text: azdata.TextComponent }; -export class BdcDashboard { +export class BdcDashboard extends BdcDashboardPage { private dashboard: azdata.workspace.ModelViewEditor; - private initialized: boolean = false; - private modelView: azdata.ModelView; private mainAreaContainer: azdata.FlexContainer; private navContainer: azdata.FlexContainer; @@ -44,8 +41,9 @@ export class BdcDashboard { private serviceTabPageMapping = new Map(); constructor(private title: string, private model: BdcDashboardModel) { - this.model.onDidUpdateBdcStatus(bdcStatus => this.handleBdcStatusUpdate(bdcStatus)); - this.model.onBdcError(errorEvent => this.handleError(errorEvent)); + super(); + this.model.onDidUpdateBdcStatus(bdcStatus => this.eventuallyRunOnInitialized(() => this.handleBdcStatusUpdate(bdcStatus))); + this.model.onBdcError(errorEvent => this.eventuallyRunOnInitialized(() => this.handleError(errorEvent))); } public showDashboard(): void { @@ -162,11 +160,10 @@ export class BdcDashboard { }); } - private handleBdcStatusUpdate(bdcStatus: BdcStatusModel): void { - if (!this.initialized || !bdcStatus) { + private handleBdcStatusUpdate(bdcStatus?: BdcStatusModel): void { + if (!bdcStatus) { return; } - this.updateServiceNavTabs(bdcStatus.services); } @@ -213,7 +210,7 @@ export class BdcDashboard { * Helper to update the navigation tabs for the services when we get a status update */ private updateServiceNavTabs(services?: ServiceStatusModel[]): void { - if (this.initialized && services) { + if (services) { // Add a nav item for each service services.forEach(s => { const existingTabPage = this.serviceTabPageMapping[s.serviceName]; diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardOverviewPage.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardOverviewPage.ts index 1b887cf13c70..90a9b9b9d999 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardOverviewPage.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardOverviewPage.ts @@ -13,6 +13,7 @@ import { EndpointModel, ServiceStatusModel, BdcStatusModel } from '../controller import { BdcDashboard } from './bdcDashboard'; import { createViewDetailsButton } from './commonControls'; import { HdfsDialogCancelledError } from './hdfsDialogBase'; +import { BdcDashboardPage } from './bdcDashboardPage'; const localize = nls.loadMessageBundle(); @@ -30,9 +31,8 @@ const serviceEndpointRowEndpointCellWidth = 350; const hyperlinkedEndpoints = [Endpoint.metricsui, Endpoint.logsui, Endpoint.sparkHistory, Endpoint.yarnUi]; -export class BdcDashboardOverviewPage { +export class BdcDashboardOverviewPage extends BdcDashboardPage { - private initialized: boolean = false; private modelBuilder: azdata.ModelBuilder; private lastUpdatedLabel: azdata.TextComponent; @@ -50,9 +50,10 @@ export class BdcDashboardOverviewPage { private serviceStatusErrorMessage: azdata.TextComponent; constructor(private dashboard: BdcDashboard, private model: BdcDashboardModel) { - this.model.onDidUpdateEndpoints(endpoints => this.handleEndpointsUpdate(endpoints)); - this.model.onDidUpdateBdcStatus(bdcStatus => this.handleBdcStatusUpdate(bdcStatus)); - this.model.onBdcError(error => this.handleBdcError(error)); + super(); + this.model.onDidUpdateEndpoints(endpoints => this.eventuallyRunOnInitialized(() => this.handleEndpointsUpdate(endpoints))); + this.model.onDidUpdateBdcStatus(bdcStatus => this.eventuallyRunOnInitialized(() => this.handleBdcStatusUpdate(bdcStatus))); + this.model.onBdcError(error => this.eventuallyRunOnInitialized(() => this.handleBdcError(error))); } public create(view: azdata.ModelView): azdata.FlexContainer { @@ -209,14 +210,12 @@ export class BdcDashboardOverviewPage { this.serviceStatusDisplayContainer.display = undefined; this.propertiesContainer.display = undefined; this.endpointsDisplayContainer.display = undefined; - - } - private handleBdcStatusUpdate(bdcStatus: BdcStatusModel): void { - if (!this.initialized || !bdcStatus) { + + private handleBdcStatusUpdate(bdcStatus?: BdcStatusModel): void { + if (!bdcStatus) { return; } - this.lastUpdatedLabel.value = localize('bdc.dashboard.lastUpdated', "Last Updated : {0}", this.model.bdcStatusLastUpdated ? @@ -237,10 +236,6 @@ export class BdcDashboardOverviewPage { } private handleEndpointsUpdate(endpoints: EndpointModel[]): void { - if (!this.initialized || !endpoints) { - return; - } - this.endpointsRowContainer.clearItems(); // Sort the endpoints. The sort method is that SQL Server Master is first - followed by all diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardPage.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardPage.ts new file mode 100644 index 000000000000..22a09faf2b8f --- /dev/null +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardPage.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Deferred } from '../../common/promise'; + +export abstract class BdcDashboardPage { + + private _initialized: boolean = false; + + private onInitializedPromise: Deferred = new Deferred(); + + constructor() { } + + protected get initialized(): boolean { + return this._initialized; + } + + protected set initialized(value: boolean) { + if (!this._initialized && value) { + this._initialized = true; + this.onInitializedPromise.resolve(); + } + } + + /** + * Runs the specified action when the component is initialized. If already initialized just runs + * the action immediately. + * @param action The action to be ran when the page is initialized + */ + protected eventuallyRunOnInitialized(action: () => void): void { + if (!this._initialized) { + this.onInitializedPromise.promise.then(() => action()).catch(error => console.error(`Unexpected error running onInitialized action for BDC Page : ${error}`)); + } else { + action(); + } + } +} + diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardResourceStatusPage.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardResourceStatusPage.ts index d50f202df551..387bf90150df 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardResourceStatusPage.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardResourceStatusPage.ts @@ -2,7 +2,6 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -'use strict'; import * as azdata from 'azdata'; import * as nls from 'vscode-nls'; @@ -12,6 +11,7 @@ import { getHealthStatusDisplayText, getHealthStatusIcon, getStateDisplayText, S import { cssStyles } from '../constants'; import { isNullOrUndefined } from 'util'; import { createViewDetailsButton } from './commonControls'; +import { BdcDashboardPage } from './bdcDashboardPage'; const localize = nls.loadMessageBundle(); @@ -39,17 +39,16 @@ const metricsAndLogsLogsColumnWidth = 75; const viewText = localize('bdc.dashboard.viewHyperlink', "View"); const notAvailableText = localize('bdc.dashboard.notAvailable', "N/A"); -export class BdcDashboardResourceStatusPage { +export class BdcDashboardResourceStatusPage extends BdcDashboardPage { private rootContainer: azdata.FlexContainer; private instanceHealthStatusRowsContainer: azdata.FlexContainer; private metricsAndLogsRowsContainer: azdata.FlexContainer; private lastUpdatedLabel: azdata.TextComponent; - private sqlMetricsComponents: azdata.Component[]; - private initialized: boolean = false; constructor(private model: BdcDashboardModel, private modelView: azdata.ModelView, private serviceName: string, private resourceName: string) { - this.model.onDidUpdateBdcStatus(bdcStatus => this.handleBdcStatusUpdate(bdcStatus)); + super(); + this.model.onDidUpdateBdcStatus(bdcStatus => this.eventuallyRunOnInitialized(() => this.handleBdcStatusUpdate(bdcStatus))); this.rootContainer = this.createContainer(modelView); } @@ -139,11 +138,14 @@ export class BdcDashboardResourceStatusPage { return rootContainer; } - private handleBdcStatusUpdate(bdcStatus: BdcStatusModel): void { + private handleBdcStatusUpdate(bdcStatus?: BdcStatusModel): void { + if (!bdcStatus) { + return; + } const service = bdcStatus.services ? bdcStatus.services.find(s => s.serviceName === this.serviceName) : undefined; const resource = service ? service.resources.find(r => r.resourceName === this.resourceName) : undefined; - if (!this.initialized || !resource || isNullOrUndefined(resource.instances)) { + if (!resource || isNullOrUndefined(resource.instances)) { return; } diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcServiceStatusPage.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcServiceStatusPage.ts index c26a202ac511..18ee6db7d1ff 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcServiceStatusPage.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcServiceStatusPage.ts @@ -2,7 +2,6 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -'use strict'; import * as azdata from 'azdata'; import { BdcStatusModel, ResourceStatusModel } from '../controller/apiGenerated'; @@ -10,12 +9,11 @@ import { BdcDashboardResourceStatusPage } from './bdcDashboardResourceStatusPage import { BdcDashboardModel } from './bdcDashboardModel'; import { getHealthStatusDot } from '../utils'; import { cssStyles } from '../constants'; +import { BdcDashboardPage } from './bdcDashboardPage'; type ServiceTab = { div: azdata.DivContainer, dot: azdata.TextComponent, text: azdata.TextComponent }; -export class BdcServiceStatusPage { - - private initialized: boolean = false; +export class BdcServiceStatusPage extends BdcDashboardPage { private currentTab: { tab: ServiceTab, index: number }; private currentTabPage: azdata.FlexContainer; @@ -25,7 +23,8 @@ export class BdcServiceStatusPage { private createdTabs: Map = new Map(); constructor(private serviceName: string, private model: BdcDashboardModel, private modelView: azdata.ModelView) { - this.model.onDidUpdateBdcStatus(bdcStatus => this.handleBdcStatusUpdate(bdcStatus)); + super(); + this.model.onDidUpdateBdcStatus(bdcStatus => this.eventuallyRunOnInitialized(() => this.handleBdcStatusUpdate(bdcStatus))); this.createPage(); } @@ -57,10 +56,9 @@ export class BdcServiceStatusPage { } private handleBdcStatusUpdate(bdcStatus: BdcStatusModel): void { - if (!this.initialized || !bdcStatus) { + if (!bdcStatus) { return; } - const service = bdcStatus.services.find(s => s.serviceName === this.serviceName); if (service && service.resources) { this.createResourceNavTabs(service.resources); @@ -79,49 +77,47 @@ export class BdcServiceStatusPage { * Helper to create the navigation tabs for the resources */ private createResourceNavTabs(resources: ResourceStatusModel[]) { - if (this.initialized) { - let tabIndex = this.createdTabs.size; - resources.forEach(resource => { - const existingTab: ServiceTab = this.createdTabs[resource.resourceName]; - if (existingTab) { - // We already created this tab so just update the status - existingTab.dot.value = getHealthStatusDot(resource.healthStatus); - } else { - // New tab - create and add to the end of the container - const currentIndex = tabIndex++; - const resourceHeaderTab = createResourceHeaderTab(this.modelView.modelBuilder, resource); - this.createdTabs[resource.resourceName] = resourceHeaderTab; - const resourceStatusPage: azdata.FlexContainer = new BdcDashboardResourceStatusPage(this.model, this.modelView, this.serviceName, resource.resourceName).container; - resourceHeaderTab.div.onDidClick(() => { - // Don't need to do anything if this is already the currently selected tab - if (this.currentTab.index === currentIndex) { - return; - } - if (this.currentTab) { - this.currentTab.tab.text.updateCssStyles(cssStyles.unselectedResourceHeaderTab); - this.resourceHeader.removeItem(this.currentTab.tab.div); - this.resourceHeader.insertItem(this.currentTab.tab.div, this.currentTab.index, { flex: '0 0 auto', CSSStyles: cssStyles.unselectedTabDiv }); - } - this.changeSelectedTabPage(resourceStatusPage); - this.currentTab = { tab: resourceHeaderTab, index: currentIndex }; - this.currentTab.tab.text.updateCssStyles(cssStyles.selectedResourceHeaderTab); - this.resourceHeader.removeItem(this.currentTab.tab.div); - this.resourceHeader.insertItem(this.currentTab.tab.div, this.currentTab.index, { flex: '0 0 auto', CSSStyles: cssStyles.selectedTabDiv }); - }); - // Set initial page - if (!this.currentTabPage) { - this.changeSelectedTabPage(resourceStatusPage); - this.currentTab = { tab: resourceHeaderTab, index: currentIndex }; - this.currentTab.tab.text.updateCssStyles(cssStyles.selectedResourceHeaderTab); - this.resourceHeader.addItem(resourceHeaderTab.div, { flex: '0 0 auto', CSSStyles: cssStyles.selectedTabDiv }); + let tabIndex = this.createdTabs.size; + resources.forEach(resource => { + const existingTab: ServiceTab = this.createdTabs[resource.resourceName]; + if (existingTab) { + // We already created this tab so just update the status + existingTab.dot.value = getHealthStatusDot(resource.healthStatus); + } else { + // New tab - create and add to the end of the container + const currentIndex = tabIndex++; + const resourceHeaderTab = createResourceHeaderTab(this.modelView.modelBuilder, resource); + this.createdTabs[resource.resourceName] = resourceHeaderTab; + const resourceStatusPage: azdata.FlexContainer = new BdcDashboardResourceStatusPage(this.model, this.modelView, this.serviceName, resource.resourceName).container; + resourceHeaderTab.div.onDidClick(() => { + // Don't need to do anything if this is already the currently selected tab + if (this.currentTab.index === currentIndex) { + return; } - else { - resourceHeaderTab.text.updateCssStyles(cssStyles.unselectedResourceHeaderTab); - this.resourceHeader.addItem(resourceHeaderTab.div, { flex: '0 0 auto', CSSStyles: cssStyles.unselectedTabDiv }); + if (this.currentTab) { + this.currentTab.tab.text.updateCssStyles(cssStyles.unselectedResourceHeaderTab); + this.resourceHeader.removeItem(this.currentTab.tab.div); + this.resourceHeader.insertItem(this.currentTab.tab.div, this.currentTab.index, { flex: '0 0 auto', CSSStyles: cssStyles.unselectedTabDiv }); } + this.changeSelectedTabPage(resourceStatusPage); + this.currentTab = { tab: resourceHeaderTab, index: currentIndex }; + this.currentTab.tab.text.updateCssStyles(cssStyles.selectedResourceHeaderTab); + this.resourceHeader.removeItem(this.currentTab.tab.div); + this.resourceHeader.insertItem(this.currentTab.tab.div, this.currentTab.index, { flex: '0 0 auto', CSSStyles: cssStyles.selectedTabDiv }); + }); + // Set initial page + if (!this.currentTabPage) { + this.changeSelectedTabPage(resourceStatusPage); + this.currentTab = { tab: resourceHeaderTab, index: currentIndex }; + this.currentTab.tab.text.updateCssStyles(cssStyles.selectedResourceHeaderTab); + this.resourceHeader.addItem(resourceHeaderTab.div, { flex: '0 0 auto', CSSStyles: cssStyles.selectedTabDiv }); } - }); - } + else { + resourceHeaderTab.text.updateCssStyles(cssStyles.unselectedResourceHeaderTab); + this.resourceHeader.addItem(resourceHeaderTab.div, { flex: '0 0 auto', CSSStyles: cssStyles.unselectedTabDiv }); + } + } + }); } }