Skip to content

Commit

Permalink
HPCC-32603 prevent Grid network races
Browse files Browse the repository at this point in the history
adds AbortController signals to the Grid's queries, so that subsequent
requests will properly abort any pending request

Signed-off-by: Jeremy Clements <[email protected]>
  • Loading branch information
jeclrsg committed Sep 13, 2024
1 parent 9699f43 commit 2330feb
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 16 deletions.
9 changes: 4 additions & 5 deletions esp/src/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion esp/src/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"@hpcc-js/chart": "2.84.1",
"@hpcc-js/codemirror": "2.63.0",
"@hpcc-js/common": "2.72.0",
"@hpcc-js/comms": "2.95.0",
"@hpcc-js/comms": "2.96.0",
"@hpcc-js/dataflow": "8.1.7",
"@hpcc-js/eclwatch": "2.75.3",
"@hpcc-js/graph": "2.86.0",
Expand Down
11 changes: 10 additions & 1 deletion esp/src/src-react/components/controls/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,21 @@ const FluentStoreGrid: React.FunctionComponent<FluentStoreGridProps> = ({
});
});

const abortController = React.useRef<AbortController>();

React.useEffect(() => {
if (abortController.current) {
abortController.current.abort({ message: "Grid aborting stale request" });
}
abortController.current = new AbortController();
}, [query]);

const refreshTable = useDeepCallback((clearSelection = false) => {
if (isNaN(start) || isNaN(count)) return;
if (clearSelection) {
selectionHandler.setItems([], true);
}
const storeQuery = store.query({ ...query }, { start, count, sort: sorted ? [sorted] : undefined });
const storeQuery = store.query({ ...query }, { start, count, sort: sorted ? [sorted] : undefined }, abortController.current.signal);
storeQuery.total.then(total => {
setTotal(total);
});
Expand Down
4 changes: 2 additions & 2 deletions esp/src/src/ESPWorkunit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1082,11 +1082,11 @@ export function CreateWUQueryStore(): BaseStore<WsWorkunitsNS.WUQuery, typeof Wo
count: "PageSize",
sortBy: "Sortby",
descending: "Descending"
}, "Wuid", request => {
}, "Wuid", (request, abortSignal) => {
if (request.Sortby && request.Sortby === "TotalClusterTime") {
request.Sortby = "ClusterTime";
}
return service.WUQuery(request).then(response => {
return service.WUQuery(request, abortSignal).then(response => {
const page = {
start: undefined,
end: undefined
Expand Down
2 changes: 1 addition & 1 deletion esp/src/src/store/Deferred.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class Deferred<T> implements Thenable<T> {
}
}

export class DeferredResponse<T> extends Deferred<T[]> {
export class DeferredResponse<T> extends Deferred<T[]> {

// --- Legacy Dojo Support (fake QeuryResults) ---
forEach(callback) {
Expand Down
6 changes: 3 additions & 3 deletions esp/src/src/store/Paged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function dataPage<T>(array: T[], start: number, size: number, total: number): Pa
return retVal;
}

type FetchData<R, T> = (query: QueryRequest<R>) => Thenable<{ data: T[], total: number }>;
type FetchData<R, T> = (query: QueryRequest<R>, abortSignal?: AbortSignal) => Thenable<{ data: T[], total: number }>;

export interface RequestFields<T extends BaseRow> {
start: keyof T;
Expand All @@ -43,7 +43,7 @@ export class Paged<R extends BaseRow = BaseRow, T extends BaseRow = BaseRow> ext
this._fetchData = fetchData;
}

fetchData(request: QueryRequest<R>, options: QueryOptions<T>): ThenableResponse<T> {
fetchData(request: QueryRequest<R>, options: QueryOptions<T>, abortSignal?: AbortSignal): ThenableResponse<T> {
if (options.start !== undefined && options.count !== undefined) {
request[this._requestFields.start] = options.start as any;
request[this._requestFields.count] = options.count as any;
Expand All @@ -52,7 +52,7 @@ export class Paged<R extends BaseRow = BaseRow, T extends BaseRow = BaseRow> ext
request[this._requestFields.sortBy] = options.sort[0].attribute as any;
request[this._requestFields.descending] = options.sort[0].descending as any;
}
return this._fetchData(request).then(response => {
return this._fetchData(request, abortSignal).then(response => {
response.data.forEach(row => {
this.index[this.getIdentity(row)] = row;
});
Expand Down
11 changes: 8 additions & 3 deletions esp/src/src/store/Store.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import * as QueryResults from "dojo/store/util/QueryResults";
import { DeferredResponse, Thenable } from "./Deferred";
import { scopedLogger } from "@hpcc-js/util";

const logger = scopedLogger("src/store/Store.ts");

// Query ---
export type Key = string | number | symbol;
Expand Down Expand Up @@ -32,19 +35,21 @@ export abstract class BaseStore<R extends BaseRow, T extends BaseRow> {
this.responseIDField = responseIDField;
}

protected abstract fetchData(request: QueryRequest<R>, options: QueryOptions<T>): ThenableResponse<T>;
protected abstract fetchData(request: QueryRequest<R>, options: QueryOptions<T>, abortSignal?: AbortSignal): ThenableResponse<T>;

abstract get(id: string | number): T;

getIdentity(object): string | number {
return object[this.responseIDField];
}

protected query(request: QueryRequest<R>, options: QueryOptions<T>): DeferredResponse<T> {
protected query(request: QueryRequest<R>, options: QueryOptions<T>, abortSignal?: AbortSignal): DeferredResponse<T> {
const retVal = new DeferredResponse<T>();
this.fetchData(request, options).then((data: QueryResponse<T>) => {
this.fetchData(request, options, abortSignal).then((data: QueryResponse<T>) => {
retVal.total.resolve(data.total);
retVal.resolve(data);
}, (err) => {
logger.debug(err);
});
return QueryResults(retVal);
}
Expand Down

0 comments on commit 2330feb

Please sign in to comment.