From 8166f83faecd0c833ca55a9e3f2612b5475cdf22 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alfonso=20Mart=C3=ADnez?= Date: Fri, 18 Sep 2020 17:16:34 +0200 Subject: [PATCH] mgr/dashboard: fix performance issue when listing large amounts of buckets MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fixes: https://tracker.ceph.com/issues/47543 Signed-off-by: Alfonso Martínez (cherry picked from commit 924368e1d0aebcb0d8f9747589d9048414d33080) Conflicts: src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.spec.ts src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.ts src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts - Adapted changes in these files to octopus code. --- qa/tasks/mgr/dashboard/test_rgw.py | 19 +++++++++++- src/pybind/mgr/dashboard/controllers/rgw.py | 14 ++++++--- .../rgw-bucket-details.component.spec.ts | 16 +++++++++- .../rgw-bucket-details.component.ts | 16 ++++++++-- .../app/shared/api/rgw-bucket.service.spec.ts | 29 ++----------------- .../src/app/shared/api/rgw-bucket.service.ts | 17 +++-------- 6 files changed, 63 insertions(+), 48 deletions(-) diff --git a/qa/tasks/mgr/dashboard/test_rgw.py b/qa/tasks/mgr/dashboard/test_rgw.py index 1e707c33d458..6f1acebece78 100644 --- a/qa/tasks/mgr/dashboard/test_rgw.py +++ b/qa/tasks/mgr/dashboard/test_rgw.py @@ -209,6 +209,20 @@ class RgwBucketTest(RgwTestCase): self.assertEqual(len(data), 1) self.assertIn('teuth-test-bucket', data) + # List all buckets with stats. + data = self._get('/api/rgw/bucket?stats=true') + self.assertStatus(200) + self.assertEqual(len(data), 1) + self.assertSchema(data[0], JObj(sub_elems={ + 'bid': JLeaf(str), + 'bucket': JLeaf(str), + 'bucket_quota': JObj(sub_elems={}, allow_unknown=True), + 'id': JLeaf(str), + 'owner': JLeaf(str), + 'usage': JObj(sub_elems={}, allow_unknown=True), + 'tenant': JLeaf(str), + }, allow_unknown=True)) + # Get the bucket. data = self._get('/api/rgw/bucket/teuth-test-bucket') self.assertStatus(200) @@ -218,7 +232,10 @@ class RgwBucketTest(RgwTestCase): 'tenant': JLeaf(str), 'bucket': JLeaf(str), 'bucket_quota': JObj(sub_elems={}, allow_unknown=True), - 'owner': JLeaf(str) + 'owner': JLeaf(str), + 'mfa_delete': JLeaf(str), + 'usage': JObj(sub_elems={}, allow_unknown=True), + 'versioning': JLeaf(str) }, allow_unknown=True)) self.assertEqual(data['bucket'], 'teuth-test-bucket') self.assertEqual(data['owner'], 'admin') diff --git a/src/pybind/mgr/dashboard/controllers/rgw.py b/src/pybind/mgr/dashboard/controllers/rgw.py index a0dc444dcabf..e9867ce34b7d 100644 --- a/src/pybind/mgr/dashboard/controllers/rgw.py +++ b/src/pybind/mgr/dashboard/controllers/rgw.py @@ -16,7 +16,7 @@ from ..services.rgw_client import RgwClient from ..tools import json_str_to_object, str_to_bool try: - from typing import List + from typing import Any, List except ImportError: # pragma: no cover pass # Just for type checking @@ -192,9 +192,15 @@ class RgwBucket(RgwRESTController): bucket_name = '{}:{}'.format(tenant, bucket_name) return bucket_name - def list(self): - # type: () -> List[str] - return self.proxy('GET', 'bucket') + def list(self, stats=False): + # type: (bool) -> List[Any] + query_params = '?stats' if stats else '' + result = self.proxy('GET', 'bucket{}'.format(query_params)) + + if stats: + result = [self._append_bid(bucket) for bucket in result] + + return result def get(self, bucket): # type: (str) -> dict diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.spec.ts index a2ede8d3f3f0..bfea7d5742e9 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.spec.ts @@ -1,8 +1,11 @@ +import { HttpClientTestingModule } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { TabsModule } from 'ngx-bootstrap/tabs'; +import { of } from 'rxjs'; import { configureTestBed, i18nProviders } from '../../../../testing/unit-test-helper'; +import { RgwBucketService } from '../../../shared/api/rgw-bucket.service'; import { CdTableSelection } from '../../../shared/models/cd-table-selection'; import { SharedModule } from '../../../shared/shared.module'; import { RgwBucketDetailsComponent } from './rgw-bucket-details.component'; @@ -10,14 +13,19 @@ import { RgwBucketDetailsComponent } from './rgw-bucket-details.component'; describe('RgwBucketDetailsComponent', () => { let component: RgwBucketDetailsComponent; let fixture: ComponentFixture; + let rgwBucketService: RgwBucketService; + let rgwBucketServiceGetSpy: jasmine.Spy; configureTestBed({ declarations: [RgwBucketDetailsComponent], - imports: [SharedModule, TabsModule.forRoot()], + imports: [SharedModule, TabsModule.forRoot(), HttpClientTestingModule], providers: [i18nProviders] }); beforeEach(() => { + rgwBucketService = TestBed.get(RgwBucketService); + rgwBucketServiceGetSpy = spyOn(rgwBucketService, 'get'); + rgwBucketServiceGetSpy.and.returnValue(of(null)); fixture = TestBed.createComponent(RgwBucketDetailsComponent); component = fixture.componentInstance; component.selection = new CdTableSelection(); @@ -27,4 +35,10 @@ describe('RgwBucketDetailsComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + it('should retrieve bucket full info', () => { + component.selection = { bid: 'bucket' }; + component.ngOnChanges(); + expect(rgwBucketServiceGetSpy).toHaveBeenCalled(); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.ts index 48f255400382..adf0b13d310d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-details/rgw-bucket-details.component.ts @@ -1,13 +1,23 @@ -import { Component, Input } from '@angular/core'; +import { Component, Input, OnChanges } from '@angular/core'; + +import { RgwBucketService } from '../../../shared/api/rgw-bucket.service'; @Component({ selector: 'cd-rgw-bucket-details', templateUrl: './rgw-bucket-details.component.html', styleUrls: ['./rgw-bucket-details.component.scss'] }) -export class RgwBucketDetailsComponent { +export class RgwBucketDetailsComponent implements OnChanges { @Input() selection: any; - constructor() {} + constructor(private rgwBucketService: RgwBucketService) {} + + ngOnChanges() { + if (this.selection) { + this.rgwBucketService.get(this.selection.bid).subscribe((bucket: object) => { + this.selection = bucket; + }); + } + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts index 0d342c48d1cd..978aca1fc4fc 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts @@ -26,33 +26,10 @@ describe('RgwBucketService', () => { expect(service).toBeTruthy(); }); - it('should call list, with enumerate returning empty', () => { - let result; - service.list().subscribe((resp) => { - result = resp; - }); - const req = httpTesting.expectOne('api/rgw/bucket'); - req.flush([]); - expect(req.request.method).toBe('GET'); - expect(result).toEqual([]); - }); - - it('should call list, with enumerate returning 2 elements', () => { - let result; - service.list().subscribe((resp) => { - result = resp; - }); - let req = httpTesting.expectOne('api/rgw/bucket'); - req.flush(['foo', 'bar']); - - req = httpTesting.expectOne('api/rgw/bucket/foo'); - req.flush({ name: 'foo' }); - - req = httpTesting.expectOne('api/rgw/bucket/bar'); - req.flush({ name: 'bar' }); - + it('should call list', () => { + service.list().subscribe(); + const req = httpTesting.expectOne('api/rgw/bucket?stats=true'); expect(req.request.method).toBe('GET'); - expect(result).toEqual([{ name: 'foo' }, { name: 'bar' }]); }); it('should call get', () => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts index 71907497d701..b73bff0dddc5 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts @@ -2,7 +2,7 @@ import { HttpClient, HttpParams } from '@angular/common/http'; import { Injectable } from '@angular/core'; import * as _ from 'lodash'; -import { forkJoin as observableForkJoin, of as observableOf } from 'rxjs'; +import { of as observableOf } from 'rxjs'; import { mergeMap } from 'rxjs/operators'; import { cdEncode } from '../decorators/cd-encode'; @@ -22,18 +22,9 @@ export class RgwBucketService { * @return {Observable} */ list() { - return this.enumerate().pipe( - mergeMap((buckets: string[]) => { - if (buckets.length > 0) { - return observableForkJoin( - buckets.map((bucket: string) => { - return this.get(bucket); - }) - ); - } - return observableOf([]); - }) - ); + let params = new HttpParams(); + params = params.append('stats', 'true'); + return this.http.get(this.url, { params: params }); } /** -- 2.47.3