Merge "Instrument scheduler to investigate slow performance"
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index 7de9701..25fb056 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -18,6 +18,8 @@
FetchRequest as FetchRequestBase,
} from '../../../../types/types';
import {ErrorCallback} from '../../../../api/rest';
+import {getAppContext} from '../../../../services/app-context';
+import {Timer} from '../../../../services/gr-reporting/gr-reporting';
import {Scheduler, Task} from '../../../../services/scheduler/scheduler';
import {RetryError} from '../../../../services/scheduler/retry-scheduler';
@@ -261,11 +263,15 @@
private readonly writeScheduler: Scheduler<Response>
) {}
- private schedule(method: string, task: Task<Response>): Promise<Response> {
+ private schedule(
+ method: string,
+ task: Task<Response>,
+ name?: string
+ ): Promise<Response> {
if (method === 'PUT' || method === 'POST' || method === 'DELETE') {
- return this.writeScheduler.schedule(task);
+ return this.writeScheduler.schedule(task, name);
} else {
- return this.readScheduler.schedule(task);
+ return this.readScheduler.schedule(task, name);
}
}
@@ -276,7 +282,41 @@
private fetchImpl(req: FetchRequest): Promise<Response> {
const method = req.fetchOptions?.method ?? HttpMethod.GET;
const startTime = Date.now();
+
+ const isWrite =
+ method === 'PUT' || method === 'POST' || method === 'DELETE';
+ const requestName = `${method} - ${req.anonymizedUrl || 'unknown'}`;
+ const debugRequestName = `${method} - ${req.anonymizedUrl || req.url}`;
+ let schedulerTimer: Timer | undefined;
+ let activeCount = 0;
+
+ if (isWrite) {
+ activeCount = this.writeScheduler.activeCount;
+ console.info(
+ '[SchedulerWait] request:',
+ debugRequestName,
+ 'count:',
+ activeCount,
+ 'active:',
+ this.writeScheduler.activeRequests
+ );
+ // 5 matches the max in flight limit of the write scheduler.
+ if (activeCount >= 5) {
+ schedulerTimer = getAppContext().reportingService.getTimer(
+ 'scheduler-waiting-time'
+ );
+ }
+ }
+
const task = async () => {
+ if (schedulerTimer) {
+ schedulerTimer.end({
+ count: activeCount,
+ request: requestName,
+ activeRequests: this.writeScheduler.activeRequests,
+ });
+ schedulerTimer = undefined;
+ }
const res = await this._auth.fetch(req.url, req.fetchOptions);
// Check for "too many requests" error and throw RetryError to cause a
// retry in this case, if the scheduler attempts retries.
@@ -284,13 +324,15 @@
return res;
};
- const resPromise = this.schedule(method, task).catch((err: unknown) => {
- if (err instanceof RetryError) {
- return err.payload;
- } else {
- throw err;
+ const resPromise = this.schedule(method, task, requestName).catch(
+ (err: unknown) => {
+ if (err instanceof RetryError) {
+ return err.payload;
+ } else {
+ throw err;
+ }
}
- });
+ );
// Log the call after it completes.
resPromise.then(res => this.logCall(req, startTime, res.status));
diff --git a/polygerrit-ui/app/services/scheduler/fake-scheduler.ts b/polygerrit-ui/app/services/scheduler/fake-scheduler.ts
index d4df3ce..c9038c9 100644
--- a/polygerrit-ui/app/services/scheduler/fake-scheduler.ts
+++ b/polygerrit-ui/app/services/scheduler/fake-scheduler.ts
@@ -9,7 +9,15 @@
export class FakeScheduler<T> implements Scheduler<T> {
readonly scheduled: Array<FakeTask> = [];
- schedule(task: Task<T>) {
+ get activeCount() {
+ return this.scheduled.length;
+ }
+
+ get activeRequests() {
+ return [];
+ }
+
+ schedule(task: Task<T>, _name?: string) {
return new Promise<T>((resolve, reject) => {
this.scheduled.push(async (error?: unknown) => {
if (error) {
diff --git a/polygerrit-ui/app/services/scheduler/max-in-flight-scheduler.ts b/polygerrit-ui/app/services/scheduler/max-in-flight-scheduler.ts
index 1febcb6..65e9e13 100644
--- a/polygerrit-ui/app/services/scheduler/max-in-flight-scheduler.ts
+++ b/polygerrit-ui/app/services/scheduler/max-in-flight-scheduler.ts
@@ -8,22 +8,36 @@
export class MaxInFlightScheduler<T> implements Scheduler<T> {
private inflight = 0;
- private waiting: Array<Task<void>> = [];
+ private waiting: Array<{task: Task<void>; name?: string}> = [];
+
+ private readonly running: string[] = [];
+
+ get activeCount(): number {
+ return this.running.length + this.waiting.length;
+ }
+
+ get activeRequests(): string[] {
+ const waitingNames = this.waiting.map(w => w.name || 'unknown');
+ return [...this.running, ...waitingNames];
+ }
constructor(
private readonly base: Scheduler<T>,
private maxInflight: number = 10
) {}
- async schedule(task: Task<T>): Promise<T> {
+ async schedule(task: Task<T>, name?: string): Promise<T> {
return new Promise<T>((resolve, reject) => {
- this.waiting.push(async () => {
- try {
- const result = await this.base.schedule(task);
- resolve(result);
- } catch (e: unknown) {
- reject(e);
- }
+ this.waiting.push({
+ task: async () => {
+ try {
+ const result = await this.base.schedule(task, name);
+ resolve(result);
+ } catch (e: unknown) {
+ reject(e);
+ }
+ },
+ name,
});
this.next();
});
@@ -32,10 +46,14 @@
private next() {
if (this.inflight >= this.maxInflight) return;
if (this.waiting.length === 0) return;
- const task = this.waiting.shift() as Task<void>;
+ const {task, name} = this.waiting.shift()!;
++this.inflight;
+ const taskName = name || 'unknown';
+ this.running.push(taskName);
task().finally(() => {
--this.inflight;
+ const index = this.running.indexOf(taskName);
+ if (index > -1) this.running.splice(index, 1);
this.next();
});
}
diff --git a/polygerrit-ui/app/services/scheduler/retry-scheduler.ts b/polygerrit-ui/app/services/scheduler/retry-scheduler.ts
index d96fc0b..67842a0 100644
--- a/polygerrit-ui/app/services/scheduler/retry-scheduler.ts
+++ b/polygerrit-ui/app/services/scheduler/retry-scheduler.ts
@@ -33,15 +33,26 @@
private backoffFactor: number = 1.618
) {}
- async schedule(task: Task<T>): Promise<T> {
+ get activeCount(): number {
+ return this.base.activeCount;
+ }
+
+ get activeRequests(): string[] {
+ return this.base.activeRequests;
+ }
+
+ async schedule(task: Task<T>, name?: string): Promise<T> {
let tries = 0;
let timeout = this.backoffIntervalMs;
const worker: Task<T> = async () => {
try {
- return await this.base.schedule(task);
+ return await this.base.schedule(task, name);
} catch (e: unknown) {
if (e instanceof RetryError && tries++ < this.maxRetry) {
+ console.info(
+ `[RetryScheduler] Retrying task, try ${tries} after ${timeout}ms`
+ );
await untilTimeout(timeout);
timeout = timeout * this.backoffFactor;
return await worker();
diff --git a/polygerrit-ui/app/services/scheduler/scheduler.ts b/polygerrit-ui/app/services/scheduler/scheduler.ts
index b834ab3..da0c3e4 100644
--- a/polygerrit-ui/app/services/scheduler/scheduler.ts
+++ b/polygerrit-ui/app/services/scheduler/scheduler.ts
@@ -5,10 +5,20 @@
*/
export type Task<T> = () => Promise<T>;
export interface Scheduler<T> {
- schedule(task: Task<T>): Promise<T>;
+ schedule(task: Task<T>, name?: string): Promise<T>;
+ get activeCount(): number;
+ get activeRequests(): string[];
}
export class BaseScheduler<T> implements Scheduler<T> {
- schedule(task: Task<T>) {
+ schedule(task: Task<T>, _name?: string) {
return task();
}
+
+ get activeCount() {
+ return 0;
+ }
+
+ get activeRequests() {
+ return [];
+ }
}
