From f107c0d09271daa33d92f2947f8dcbdb0e8d276f Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Wed, 17 Oct 2018 10:16:49 +0100 Subject: [PATCH 1/2] Update app instance cell data when scaling up - Every time we scale up app instances we now refresh the data we use to determine which cell the app instance is in - This helps avoid situations where user scales down and then scales up and old instance cell data is used - Unfortunately the firehose only emits this information every 60 seconds, so if the scale down + up happens within this time the old data is still used. There's not much we can do about this without lots of code --- .../cf-app-instances-config.service.ts | 16 +++++++------ src/frontend/app/store/effects/app.effects.ts | 23 ++++++++++++++++--- .../app/store/reducers/routes.reducer.ts | 9 ++++---- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/frontend/app/shared/components/list/list-types/app-instance/cf-app-instances-config.service.ts b/src/frontend/app/shared/components/list/list-types/app-instance/cf-app-instances-config.service.ts index dc82e805a8..5bfd50d3c9 100644 --- a/src/frontend/app/shared/components/list/list-types/app-instance/cf-app-instances-config.service.ts +++ b/src/frontend/app/shared/components/list/list-types/app-instance/cf-app-instances-config.service.ts @@ -25,6 +25,14 @@ import { CfAppInstancesDataSource } from './cf-app-instances-data-source'; import { TableCellCfCellComponent } from './table-cell-cf-cell/table-cell-cf-cell.component'; import { TableCellUsageComponent } from './table-cell-usage/table-cell-usage.component'; +export function createAppInstancesMetricAction(appGuid: string, cfGuid: string): FetchApplicationMetricsAction { + return new FetchApplicationMetricsAction( + appGuid, + cfGuid, + new MetricQueryConfig('firehose_container_metric_cpu_percentage'), + MetricQueryType.QUERY + ); +} @Injectable() export class CfAppInstancesConfigService implements IListConfig { @@ -215,12 +223,7 @@ export class CfAppInstancesConfigService implements IListConfig getInitialised = () => this.initialised$; private createMetricsResults(entityServiceFactory: EntityServiceFactory) { - const metricsAction = new FetchApplicationMetricsAction( - this.appService.appGuid, - this.appService.cfGuid, - new MetricQueryConfig('firehose_container_metric_cpu_percentage'), - MetricQueryType.QUERY - ); + const metricsAction = createAppInstancesMetricAction(this.appService.appGuid, this.appService.cfGuid); return entityServiceFactory.create>>( metricSchemaKey, entityFactory(metricSchemaKey), @@ -229,5 +232,4 @@ export class CfAppInstancesConfigService implements IListConfig false ); } - } diff --git a/src/frontend/app/store/effects/app.effects.ts b/src/frontend/app/store/effects/app.effects.ts index 9e54583919..11e6ca1322 100644 --- a/src/frontend/app/store/effects/app.effects.ts +++ b/src/frontend/app/store/effects/app.effects.ts @@ -3,8 +3,12 @@ import { Actions, Effect } from '@ngrx/effects'; import { Store } from '@ngrx/store'; import { map } from 'rxjs/operators'; +import { + createAppInstancesMetricAction, +} from '../../shared/components/list/list-types/app-instance/cf-app-instances-config.service'; import { GetAppSummaryAction } from '../actions/app-metadata.actions'; -import { ASSIGN_ROUTE, AssociateRouteWithAppApplication, ASSIGN_ROUTE_SUCCESS } from '../actions/application-service-routes.actions'; +import { ASSIGN_ROUTE_SUCCESS } from '../actions/application-service-routes.actions'; +import { UPDATE_SUCCESS, UpdateExistingApplication } from '../actions/application.actions'; import { AppState } from '../app-state'; import { APISuccessOrFailedAction } from '../types/request.types'; @@ -17,10 +21,23 @@ export class AppEffects { private store: Store, ) { } - @Effect({ dispatch: false }) upateSummary$ = this.actions$.ofType(ASSIGN_ROUTE_SUCCESS).pipe( + @Effect({ dispatch: false }) updateSummary$ = this.actions$.ofType(ASSIGN_ROUTE_SUCCESS).pipe( map(action => { - this.store.dispatch(new GetAppSummaryAction(action.apiAction.guid, action.apiAction.endpointGuid)); + this.store.dispatch(new GetAppSummaryAction(action.apiAction.guid, action.apiAction.endpointGuid)); }), + ); + @Effect({ dispatch: false }) clearCellMetrics$ = this.actions$.ofType(UPDATE_SUCCESS).pipe( + map(action => { + // User's can scale down instances and previous instance data is kept in store, when the user scales up again this stale data can + // be incorrectly shown straight away. In order to work around this fetch the latest metrics again when scaling up + // Note - If this happens within the metrics update time period (60 seconds) the stale one is returned again, unfortunately there's + // no way to work around this. + const updateAction: UpdateExistingApplication = action.apiAction as UpdateExistingApplication; + if (updateAction.newApplication.instances > updateAction.existingApplication.instances) { + const metricsAction = createAppInstancesMetricAction(updateAction.guid, updateAction.endpointGuid); + this.store.dispatch(metricsAction); + } + }), ); } diff --git a/src/frontend/app/store/reducers/routes.reducer.ts b/src/frontend/app/store/reducers/routes.reducer.ts index 77aa5fc3b2..97e6b1327e 100644 --- a/src/frontend/app/store/reducers/routes.reducer.ts +++ b/src/frontend/app/store/reducers/routes.reducer.ts @@ -1,10 +1,9 @@ -import { AppState, IRequestEntityTypeState } from '../app-state'; -import { Action } from '@ngrx/store'; +import { IAppSummary, IRoute } from '../../core/cf-api.types'; +import { ASSIGN_ROUTE_SUCCESS, AssociateRouteWithAppApplication } from '../actions/application-service-routes.actions'; +import { DeleteRoute, RouteEvents, UnmapRoute } from '../actions/route.actions'; +import { IRequestEntityTypeState } from '../app-state'; import { APIResource } from '../types/api.types'; -import { RouteEvents, UnmapRoute, DeleteRoute } from '../actions/route.actions'; import { APISuccessOrFailedAction } from '../types/request.types'; -import { IRoute, IAppSummary } from '../../core/cf-api.types'; -import { ASSIGN_ROUTE_SUCCESS, AssociateRouteWithAppApplication } from '../actions/application-service-routes.actions'; export function routeReducer(state: IRequestEntityTypeState>, action: APISuccessOrFailedAction) { switch (action.type) { From 0e25aa648f9ad1d16bd1f0e67cbf8c87be8e6274 Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Tue, 30 Oct 2018 14:34:51 +0000 Subject: [PATCH 2/2] Don't update metrics when there's no metrics endpoint associated with cf --- .../app/features/endpoints/endpoint-helpers.ts | 14 ++++++++++++++ src/frontend/app/store/effects/app.effects.ts | 11 ++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/frontend/app/features/endpoints/endpoint-helpers.ts b/src/frontend/app/features/endpoints/endpoint-helpers.ts index 5ea24a3390..fa3a51d84c 100644 --- a/src/frontend/app/features/endpoints/endpoint-helpers.ts +++ b/src/frontend/app/features/endpoints/endpoint-helpers.ts @@ -1,4 +1,11 @@ +import { Store } from '@ngrx/store'; +import { Observable } from 'rxjs'; +import { first, map } from 'rxjs/operators'; + import { urlValidationExpression } from '../../core/utils.service'; +import { AppState } from '../../store/app-state'; +import { endpointSchemaKey } from '../../store/helpers/entity-factory'; +import { selectEntities } from '../../store/selectors/api.selectors'; import { EndpointModel, EndpointType } from './../../store/types/endpoint.types'; export function getFullEndpointApiUrl(endpoint: EndpointModel) { @@ -72,3 +79,10 @@ export function getIconForEndpoint(type: string): EndpointIcon { } return icon; } + +export function endpointHasMetrics(endpointGuid: string, store: Store): Observable { + return store.select(selectEntities(endpointSchemaKey)).pipe( + first(), + map(state => !!state[endpointGuid].metadata && !!state[endpointGuid].metadata.metrics) + ); +} diff --git a/src/frontend/app/store/effects/app.effects.ts b/src/frontend/app/store/effects/app.effects.ts index 11e6ca1322..0ce65b99fe 100644 --- a/src/frontend/app/store/effects/app.effects.ts +++ b/src/frontend/app/store/effects/app.effects.ts @@ -1,8 +1,9 @@ import { Injectable } from '@angular/core'; import { Actions, Effect } from '@ngrx/effects'; import { Store } from '@ngrx/store'; -import { map } from 'rxjs/operators'; +import { first, map } from 'rxjs/operators'; +import { endpointHasMetrics } from '../../features/endpoints/endpoint-helpers'; import { createAppInstancesMetricAction, } from '../../shared/components/list/list-types/app-instance/cf-app-instances-config.service'; @@ -35,8 +36,12 @@ export class AppEffects { // no way to work around this. const updateAction: UpdateExistingApplication = action.apiAction as UpdateExistingApplication; if (updateAction.newApplication.instances > updateAction.existingApplication.instances) { - const metricsAction = createAppInstancesMetricAction(updateAction.guid, updateAction.endpointGuid); - this.store.dispatch(metricsAction); + // First check that we have a metrics endpoint associated with this cf + endpointHasMetrics(updateAction.endpointGuid, this.store).pipe(first()).subscribe(hasMetrics => { + if (hasMetrics) { + this.store.dispatch(createAppInstancesMetricAction(updateAction.guid, updateAction.endpointGuid)); + } + }); } }), );