Skip to content

Commit 43358b3

Browse files
committed
fix: review fixes
1 parent 40fc79b commit 43358b3

File tree

7 files changed

+119
-58
lines changed

7 files changed

+119
-58
lines changed

src/components/QueryExecutionStatus/QueryExecutionStatus.tsx

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import type {LabelProps, TextProps} from '@gravity-ui/uikit';
66
import {Icon, Label, Spin, Text} from '@gravity-ui/uikit';
77

88
import {isQueryCancelledError} from '../../containers/Tenant/Query/utils/isQueryCancelledError';
9-
import {selectQueryDuration, setQueryDuration} from '../../store/reducers/query/query';
9+
import {selectQueryDuration} from '../../store/reducers/query/query';
1010
import {cn} from '../../utils/cn';
1111
import {HOUR_IN_SECONDS, SECOND_IN_MS} from '../../utils/constants';
12-
import {useTypedDispatch, useTypedSelector} from '../../utils/hooks';
12+
import {useTypedSelector} from '../../utils/hooks';
1313
import {isAxiosError} from '../../utils/response';
1414

1515
import './QueryExecutionStatus.scss';
@@ -27,33 +27,36 @@ export const QueryExecutionStatus = ({className, error, loading}: QueryExecution
2727
let label: string;
2828
let theme: LabelProps['theme'];
2929
let textColor: TextProps['color'];
30-
const dispatch = useTypedDispatch();
31-
const queryDuration = useTypedSelector(selectQueryDuration);
30+
const {startTime, endTime} = useTypedSelector(selectQueryDuration);
31+
32+
const [queryDuration, setQueryDuration] = React.useState<number>(
33+
startTime ? (endTime || Date.now()) - startTime : 0,
34+
);
3235

3336
const isCancelled = isQueryCancelledError(error);
3437

38+
const setDuration = React.useCallback(() => {
39+
if (startTime) {
40+
const actualEndTime = endTime || Date.now();
41+
setQueryDuration(actualEndTime - startTime);
42+
}
43+
}, [endTime, startTime]);
44+
3545
React.useEffect(() => {
3646
let timerId: ReturnType<typeof setInterval> | undefined;
37-
let startTime = Date.now();
47+
setDuration();
3848

3949
if (loading) {
40-
dispatch(setQueryDuration(0));
41-
startTime = Date.now();
42-
timerId = setInterval(() => {
43-
dispatch(setQueryDuration(Date.now() - startTime));
44-
}, SECOND_IN_MS);
50+
timerId = setInterval(setDuration, SECOND_IN_MS);
4551
} else {
4652
clearInterval(timerId);
4753
}
4854
return () => {
4955
clearInterval(timerId);
5056
};
51-
}, [dispatch, loading]);
57+
}, [loading, setDuration]);
5258

5359
const formattedQueryDuration = React.useMemo(() => {
54-
if (!queryDuration) {
55-
return duration(0).format('mm:ss');
56-
}
5760
return queryDuration > HOUR_IN_SECONDS * SECOND_IN_MS
5861
? duration(queryDuration).format('hh:mm:ss')
5962
: duration(queryDuration).format('mm:ss');

src/containers/Tenant/Query/QueryEditor/QueryEditor.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {QueryResultViewer} from '../QueryResult/QueryResultViewer';
5151
import {QuerySettingsDialog} from '../QuerySettingsDialog/QuerySettingsDialog';
5252

5353
import {YqlEditor} from './YqlEditor';
54+
import {queryManagerInstance} from './helpers';
5455

5556
import './QueryEditor.scss';
5657

@@ -97,8 +98,6 @@ export default function QueryEditor(props: QueryEditorProps) {
9798
const [sendQuery] = queryApi.useUseSendQueryMutation();
9899
const [streamQuery] = queryApi.useUseStreamQueryMutation();
99100

100-
const runningQueryRef = React.useRef<{abort: VoidFunction} | null>(null);
101-
102101
const tableSettings = React.useMemo(() => {
103102
return isStreamingEnabled
104103
? {
@@ -142,22 +141,26 @@ export default function QueryEditor(props: QueryEditorProps) {
142141
const queryId = uuidv4();
143142

144143
if (isStreamingEnabled) {
145-
runningQueryRef.current = streamQuery({
144+
const query = streamQuery({
146145
actionType: 'execute',
147146
query: text,
148147
database: tenantName,
149148
querySettings,
150149
enableTracingLevel,
151150
});
151+
152+
queryManagerInstance.registerQuery(query);
152153
} else {
153-
runningQueryRef.current = sendQuery({
154+
const query = sendQuery({
154155
actionType: 'execute',
155156
query: text,
156157
database: tenantName,
157158
querySettings,
158159
enableTracingLevel,
159160
queryId,
160161
});
162+
163+
queryManagerInstance.registerQuery(query);
161164
}
162165

163166
dispatch(setShowPreview(false));
@@ -185,7 +188,7 @@ export default function QueryEditor(props: QueryEditorProps) {
185188

186189
const queryId = uuidv4();
187190

188-
runningQueryRef.current = sendQuery({
191+
const query = sendQuery({
189192
actionType: 'explain',
190193
query: text,
191194
database: tenantName,
@@ -194,6 +197,8 @@ export default function QueryEditor(props: QueryEditorProps) {
194197
queryId,
195198
});
196199

200+
queryManagerInstance.registerQuery(query);
201+
197202
dispatch(setShowPreview(false));
198203

199204
dispatchResultVisibilityState(PaneVisibilityActionTypes.triggerExpand);
@@ -221,7 +226,6 @@ export default function QueryEditor(props: QueryEditorProps) {
221226
tenantName={tenantName}
222227
queryId={result?.queryId}
223228
isStreamingEnabled={isStreamingEnabled}
224-
runningQueryRef={runningQueryRef}
225229
/>
226230
);
227231
};

src/containers/Tenant/Query/QueryEditor/helpers.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,24 @@ export function useCodeAssistHelpers() {
114114
monacoGhostConfig,
115115
};
116116
}
117+
118+
class QueryManager {
119+
private query: {abort: VoidFunction} | null;
120+
121+
constructor() {
122+
this.query = null;
123+
}
124+
125+
registerQuery(query: {abort: VoidFunction}) {
126+
this.query = query;
127+
}
128+
129+
abortQuery() {
130+
if (this.query) {
131+
this.query.abort();
132+
this.query = null;
133+
}
134+
}
135+
}
136+
137+
export const queryManagerInstance = new QueryManager();

src/containers/Tenant/Query/QueryEditorControls/QueryEditorControls.tsx

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {cn} from '../../../../utils/cn';
77
import createToast from '../../../../utils/createToast';
88
import {useTypedSelector} from '../../../../utils/hooks';
99
import {NewSQL} from '../NewSQL/NewSQL';
10+
import {queryManagerInstance} from '../QueryEditor/helpers';
1011
import {SaveQuery} from '../SaveQuery/SaveQuery';
1112
import i18n from '../i18n';
1213

@@ -23,7 +24,6 @@ interface QueryEditorControlsProps {
2324
queryId?: string;
2425
tenantName: string;
2526
isStreamingEnabled?: boolean;
26-
runningQueryRef: React.MutableRefObject<{abort: VoidFunction} | null>;
2727

2828
handleGetExplainQueryClick: (text: string) => void;
2929
handleSendExecuteClick: (text: string) => void;
@@ -75,38 +75,21 @@ export const QueryEditorControls = ({
7575
queryId,
7676
tenantName,
7777
isStreamingEnabled,
78-
runningQueryRef,
7978

8079
handleSendExecuteClick,
8180
onSettingsButtonClick,
8281
handleGetExplainQueryClick,
8382
}: QueryEditorControlsProps) => {
8483
const input = useTypedSelector(selectUserInput);
8584
const [sendCancelQuery, cancelQueryResponse] = cancelQueryApi.useCancelQueryMutation();
86-
const stopButtonAppearRef = React.useRef<number>(0);
87-
const [isStoppable, setIsStoppable] = React.useState(false);
88-
89-
React.useEffect(() => {
90-
if (isLoading) {
91-
stopButtonAppearRef.current = window.setTimeout(() => {
92-
setIsStoppable(true);
93-
}, STOP_APPEAR_TIMEOUT);
94-
} else {
95-
window.clearTimeout(stopButtonAppearRef.current);
96-
setIsStoppable(false);
97-
cancelQueryResponse.reset();
98-
}
99-
100-
return () => {
101-
window.clearTimeout(stopButtonAppearRef.current);
102-
};
103-
}, [cancelQueryResponse, isLoading]);
85+
const [isStoppable, setIsStoppable] = React.useState(isLoading);
86+
const stopButtonAppearRef = React.useRef<number | null>(null);
10487

10588
const onStopButtonClick = React.useCallback(async () => {
10689
if (queryId) {
10790
try {
108-
if (isStreamingEnabled && runningQueryRef.current) {
109-
runningQueryRef.current.abort();
91+
if (isStreamingEnabled) {
92+
queryManagerInstance.abortQuery();
11093
} else if (queryId) {
11194
await sendCancelQuery({queryId, database: tenantName}).unwrap();
11295
}
@@ -120,18 +103,39 @@ export const QueryEditorControls = ({
120103
});
121104
}
122105
}
123-
}, [isStreamingEnabled, queryId, runningQueryRef, sendCancelQuery, tenantName]);
106+
}, [isStreamingEnabled, queryId, sendCancelQuery, tenantName]);
124107

125108
const isRunHighlighted = highlightedAction === 'execute';
126109
const isExplainHighlighted = highlightedAction === 'explain';
127110

128-
const onRunButtonClick = () => {
111+
const runSetStoppableTimeout = React.useCallback(() => {
112+
if (stopButtonAppearRef.current) {
113+
window.clearTimeout(stopButtonAppearRef.current);
114+
}
115+
116+
setIsStoppable(false);
117+
stopButtonAppearRef.current = window.setTimeout(() => {
118+
setIsStoppable(true);
119+
}, STOP_APPEAR_TIMEOUT);
120+
}, []);
121+
122+
const onRunButtonClick = React.useCallback(() => {
129123
handleSendExecuteClick(input);
130-
};
124+
runSetStoppableTimeout();
125+
}, [handleSendExecuteClick, input, runSetStoppableTimeout]);
131126

132-
const onExplainButtonClick = () => {
127+
const onExplainButtonClick = React.useCallback(() => {
133128
handleGetExplainQueryClick(input);
134-
};
129+
runSetStoppableTimeout();
130+
}, [handleGetExplainQueryClick, input, runSetStoppableTimeout]);
131+
132+
React.useEffect(() => {
133+
return () => {
134+
if (stopButtonAppearRef.current) {
135+
window.clearTimeout(stopButtonAppearRef.current);
136+
}
137+
};
138+
}, []);
135139

136140
const controlsDisabled = disabled || !input;
137141

src/store/reducers/query/query.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ const slice = createSlice({
5353
setQueryResult: (state, action: PayloadAction<QueryResult | undefined>) => {
5454
state.result = action.payload;
5555
},
56-
setQueryDuration: (state, action: PayloadAction<number>) => {
57-
state.queryDuration = action.payload;
58-
},
5956
saveQueryToHistory: (
6057
state,
6158
action: PayloadAction<{queryText: string; queryId: string}>,
@@ -134,7 +131,10 @@ const slice = createSlice({
134131
selectors: {
135132
selectQueriesHistoryFilter: (state) => state.history.filter || '',
136133
selectTenantPath: (state) => state.tenantPath,
137-
selectQueryDuration: (state) => state.queryDuration,
134+
selectQueryDuration: (state) => ({
135+
startTime: state.result?.startTime,
136+
endTime: state.result?.endTime,
137+
}),
138138
selectResult: (state) => state.result,
139139
selectQueriesHistory: (state) => {
140140
const items = state.history.queries;
@@ -153,7 +153,6 @@ export default slice.reducer;
153153
export const {
154154
changeUserInput,
155155
setQueryResult,
156-
setQueryDuration,
157156
saveQueryToHistory,
158157
updateQueryInHistory,
159158
goToPreviousQuery,
@@ -202,7 +201,15 @@ export const queryApi = api.injectEndpoints({
202201
{query, database, querySettings = {}, enableTracingLevel},
203202
{signal, dispatch, getState},
204203
) => {
205-
dispatch(setQueryResult({type: 'execute', queryId: '', isLoading: true}));
204+
const startTime = Date.now();
205+
dispatch(
206+
setQueryResult({
207+
type: 'execute',
208+
queryId: '',
209+
isLoading: true,
210+
startTime,
211+
}),
212+
);
206213

207214
const {action, syntax} = getActionAndSyntaxFromQueryMode(
208215
'execute',
@@ -247,18 +254,21 @@ export const queryApi = api.injectEndpoints({
247254
},
248255
{
249256
signal,
250-
onQueryResponseChunk: (chunk) => {
251-
dispatch(setStreamQueryResponse(chunk));
252-
},
257+
// First chunk is session chunk
253258
onSessionChunk: (chunk) => {
254259
dispatch(setStreamSession(chunk));
255260
},
261+
// Data chunks follow session chunk
256262
onStreamDataChunk: (chunk) => {
257263
streamDataChunkBatch.push(chunk);
258264
if (!batchTimeout) {
259265
batchTimeout = window.requestAnimationFrame(flushBatch);
260266
}
261267
},
268+
// Last chunk is query response chunk
269+
onQueryResponseChunk: (chunk) => {
270+
dispatch(setStreamQueryResponse(chunk));
271+
},
262272
},
263273
);
264274

@@ -277,6 +287,8 @@ export const queryApi = api.injectEndpoints({
277287
type: 'execute',
278288
error,
279289
isLoading: false,
290+
startTime,
291+
endTime: Date.now(),
280292
queryId: state.query.result?.queryId || '',
281293
}),
282294
);
@@ -296,7 +308,15 @@ export const queryApi = api.injectEndpoints({
296308
},
297309
{signal, dispatch},
298310
) => {
299-
dispatch(setQueryResult({type: actionType, queryId, isLoading: true}));
311+
const startTime = Date.now();
312+
dispatch(
313+
setQueryResult({
314+
type: actionType,
315+
queryId,
316+
isLoading: true,
317+
startTime,
318+
}),
319+
);
300320

301321
const {action, syntax} = getActionAndSyntaxFromQueryMode(
302322
actionType,
@@ -338,6 +358,8 @@ export const queryApi = api.injectEndpoints({
338358
error: response,
339359
isLoading: false,
340360
queryId,
361+
startTime,
362+
endTime: Date.now(),
341363
}),
342364
);
343365
return {error: response};
@@ -367,6 +389,8 @@ export const queryApi = api.injectEndpoints({
367389
data,
368390
isLoading: false,
369391
queryId,
392+
startTime,
393+
endTime: Date.now(),
370394
}),
371395
);
372396
return {data: null};
@@ -377,6 +401,8 @@ export const queryApi = api.injectEndpoints({
377401
error,
378402
isLoading: false,
379403
queryId,
404+
startTime,
405+
endTime: Date.now(),
380406
}),
381407
);
382408
return {error};

src/store/reducers/query/streamingReducers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export const setStreamQueryResponse = (
5555
state.result.data.plan = chunk.plan;
5656
state.result.data.stats = chunk.stats;
5757
}
58+
59+
state.result.endTime = Date.now();
5860
};
5961

6062
const getEmptyResultSet = () => {

0 commit comments

Comments
 (0)