Skip to content

Commit b22cdc4

Browse files
shanghaikidshanghaikid
authored
fix: server crash and memory leak after switching between databases (zilliztech#652)
* fix: server crash and memory leak after switching between databases Signed-off-by: ryjiang <[email protected]> * WIP Signed-off-by: ryjiang <[email protected]> * make sure all requests with database Signed-off-by: shanghaikid <[email protected]> * clean console Signed-off-by: shanghaikid <[email protected]> * clean console Signed-off-by: shanghaikid <[email protected]> --------- Signed-off-by: ryjiang <[email protected]> Signed-off-by: shanghaikid <[email protected]>
1 parent 73f29ca commit b22cdc4

File tree

16 files changed

+316
-136
lines changed

16 files changed

+316
-136
lines changed

‎client/src/components/layout/GlobalEffect.tsx‎

Lines changed: 63 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,78 @@
1-
import React, { useContext } from 'react';
1+
import React, { useContext, useEffect } from 'react';
22
import axiosInstance from '@/http/Axios';
3-
import { rootContext, authContext } from '@/context';
3+
import { rootContext, authContext, dataContext } from '@/context';
44
import { HTTP_STATUS_CODE } from '@server/utils/Const';
55

66
let axiosResInterceptor: number | null = null;
7-
// let timer: Record<string, ReturnType<typeof setTimeout> | number>[] = [];
8-
// we only take side effect here, nothing else
9-
const GlobalEffect = (props: { children: React.ReactNode }) => {
7+
8+
const GlobalEffect = ({ children }: { children: React.ReactNode }) => {
109
const { openSnackBar } = useContext(rootContext);
1110
const { logout } = useContext(authContext);
11+
const { database } = useContext(dataContext);
1212

13-
// catch axios error here
14-
if (axiosResInterceptor === null) {
15-
axiosResInterceptor = axiosInstance.interceptors.response.use(
16-
function (res: any) {
17-
if (res.statusCode && res.statusCode !== HTTP_STATUS_CODE.OK) {
18-
openSnackBar(res.data.message, 'warning');
19-
return Promise.reject(res.data);
20-
}
21-
22-
return res;
13+
useEffect(() => {
14+
// Add database header to all axios requests
15+
const requestInterceptor = axiosInstance.interceptors.request.use(
16+
config => {
17+
config.headers['x-attu-database'] = database;
18+
return config;
2319
},
24-
function (error: any) {
25-
const { response = {} } = error;
26-
27-
switch (response.status) {
28-
case HTTP_STATUS_CODE.UNAUTHORIZED:
29-
case HTTP_STATUS_CODE.FORBIDDEN:
30-
setTimeout(logout, 1000);
31-
break;
32-
default:
33-
break;
34-
}
35-
if (response.data) {
36-
const { message: errMsg } = response.data;
37-
// We need check status 401 in login page
38-
// So server will return 500 when change the user password.
39-
errMsg && openSnackBar(errMsg, 'error');
40-
return Promise.reject(error);
41-
}
42-
if (error.message) {
20+
error => Promise.reject(error)
21+
);
22+
23+
// Clean up interceptor on unmount
24+
return () => {
25+
axiosInstance.interceptors.request.eject(requestInterceptor);
26+
};
27+
}, [database]);
28+
29+
useEffect(() => {
30+
if (axiosResInterceptor === null) {
31+
axiosResInterceptor = axiosInstance.interceptors.response.use(
32+
(response: any) => {
33+
if (
34+
response.statusCode &&
35+
response.statusCode !== HTTP_STATUS_CODE.OK
36+
) {
37+
openSnackBar(response.data.message, 'warning');
38+
return Promise.reject(response.data);
39+
}
40+
return response;
41+
},
42+
error => {
43+
const { response } = error;
44+
if (response) {
45+
switch (response.status) {
46+
case HTTP_STATUS_CODE.UNAUTHORIZED:
47+
case HTTP_STATUS_CODE.FORBIDDEN:
48+
setTimeout(() => logout(true), 1000);
49+
break;
50+
default:
51+
break;
52+
}
53+
const errorMessage = response.data?.message;
54+
if (errorMessage) {
55+
openSnackBar(errorMessage, 'error');
56+
return Promise.reject(error);
57+
}
58+
}
59+
// Handle other error cases
4360
openSnackBar(error.message, 'error');
61+
return Promise.reject(error);
4462
}
45-
return Promise.reject(error);
63+
);
64+
}
65+
66+
// Clean up response interceptor on unmount
67+
return () => {
68+
if (axiosResInterceptor !== null) {
69+
axiosInstance.interceptors.response.eject(axiosResInterceptor);
70+
axiosResInterceptor = null;
4671
}
47-
);
48-
}
49-
// get global data
72+
};
73+
}, [logout, openSnackBar]);
5074

51-
return <>{props.children}</>;
75+
return <>{children}</>;
5276
};
5377

5478
export default GlobalEffect;

‎client/src/components/layout/Header.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ const Header: FC = () => {
106106
};
107107

108108
const handleLogout = async () => {
109-
logout();
109+
logout(false);
110110
};
111111

112112
const useDatabase = async (database: string) => {

‎client/src/context/Auth.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ export const AuthProvider = (props: { children: React.ReactNode }) => {
6969
return res;
7070
};
7171
// logout API
72-
const logout = () => {
72+
const logout = async (pass?: boolean) => {
73+
if (!pass) {
74+
// close connetion
75+
await MilvusService.closeConnection();
76+
}
7377
// clear client id
7478
setClientId('');
7579
// remove client id from local storage

‎client/src/context/Data.tsx‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,23 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
136136

137137
// Websocket Callback: update single collection
138138
const updateCollections = useCallback(
139-
(updateCollections: CollectionFullObject[]) => {
139+
(props: { collections: CollectionFullObject[]; database?: string }) => {
140+
const { collections, database: remote } = props;
141+
if (
142+
remote !== database &&
143+
database !== undefined &&
144+
remote !== undefined
145+
) {
146+
// console.log('database not matched', remote, database);
147+
return;
148+
}
140149
// check state to see if it is loading or building index, if so, start server cron job
141-
detectLoadingIndexing(updateCollections);
150+
detectLoadingIndexing(collections);
142151
// update single collection
143152
setCollections(prev => {
144153
// update exist collection
145154
const newCollections = prev.map(v => {
146-
const collectionToUpdate = updateCollections.find(c => c.id === v.id);
155+
const collectionToUpdate = collections.find(c => c.id === v.id);
147156

148157
if (collectionToUpdate) {
149158
return collectionToUpdate;
@@ -215,7 +224,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
215224
const res = await CollectionService.getCollection(name);
216225

217226
// update collection
218-
updateCollections([res]);
227+
updateCollections({ collections: [res] });
219228

220229
return res;
221230
};
@@ -260,7 +269,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
260269
}
261270

262271
// update collection, and trigger cron job
263-
updateCollections([collection]);
272+
updateCollections({ collections: [collection] });
264273
};
265274

266275
// API: release collection
@@ -275,7 +284,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
275284
const newCollection = await CollectionService.renameCollection(name, {
276285
new_collection_name: newName,
277286
});
278-
updateCollections([newCollection]);
287+
updateCollections({ collections: [newCollection] });
279288

280289
return newCollection;
281290
};
@@ -307,7 +316,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
307316
// create index
308317
const newCollection = await CollectionService.createIndex(param);
309318
// update collection
310-
updateCollections([newCollection]);
319+
updateCollections({ collections: [newCollection] });
311320

312321
return newCollection;
313322
};
@@ -317,7 +326,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
317326
// drop index
318327
const { data } = await CollectionService.dropIndex(params);
319328
// update collection
320-
updateCollections([data]);
329+
updateCollections({ collections: [data] });
321330

322331
return data;
323332
};
@@ -329,7 +338,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
329338
alias,
330339
});
331340
// update collection
332-
updateCollections([newCollection]);
341+
updateCollections({ collections: [newCollection] });
333342

334343
return newCollection;
335344
};
@@ -342,7 +351,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
342351
});
343352

344353
// update collection
345-
updateCollections([data]);
354+
updateCollections({ collections: [data] });
346355

347356
return data;
348357
};
@@ -359,7 +368,7 @@ export const DataProvider = (props: { children: React.ReactNode }) => {
359368
});
360369

361370
// update existing collection
362-
updateCollections([newCollection]);
371+
updateCollections({ collections: [newCollection] });
363372

364373
return newCollection;
365374
};

‎client/src/context/Types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export type AuthContextType = {
7070
clientId: string;
7171
isManaged: boolean;
7272
isAuth: boolean;
73-
logout: () => void;
73+
logout: (pass?: boolean) => void;
7474
login: (params: AuthReq) => Promise<AuthObject>;
7575
};
7676

‎server/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
},
1414
"dependencies": {
1515
"@json2csv/plainjs": "^7.0.3",
16-
"@zilliz/milvus2-sdk-node": "^2.4.8",
16+
"@zilliz/milvus2-sdk-node": "^2.4.9",
1717
"axios": "^1.7.4",
1818
"chalk": "4.1.2",
1919
"class-sanitizer": "^1.0.1",

0 commit comments

Comments
 (0)