Fix for concurrency problem in Elasticsearch and fix for excluding invalid mountnames. Related issues are 1472, 901 and 1455#1495
Conversation
There was a problem hiding this comment.
Hi karthikeyan,
Thanks for working out in detail.
The proposed solution looks good. After going through the changes I have some doubts regarding (considering that the prod environment produce millions of alarms to MWDI consumer).
- memory leak &
- error isolation
feel free to either fix or comment further. Many thanks.
|
|
||
| return result; | ||
| } finally { | ||
| release(); |
There was a problem hiding this comment.
good to release the lock in the finally block.
| let alarmTypeQualifier = currentJSON['alarm-type-qualifier']; | ||
| let problemSeverity = currentJSON['problem-severity']; | ||
| let mountname = decodeMountName(resource, false); | ||
| // 🔹 Wrap the critical section with a per-mountname lock |
There was a problem hiding this comment.
Kindly remove the diamond symbol. Thanks.
.gitignore
Outdated
| @@ -1 +1 @@ | |||
|
|
|||
| .idea/ | |||
There was a problem hiding this comment.
If this is specific to local environment. Please remove. Thanks.
| return result; | ||
| } finally { | ||
| release(); | ||
| queueCounts.set(id, queueCounts.get(id) - 1); |
There was a problem hiding this comment.
We're storing a Promise and a queue count for each mountname (id) in locks and queueCounts, but never removing them after processing is complete.
As MWDI already have memory consumption problems, suggesting to clean up both maps when the queue count drops to zero. Thanks.
finally {
release();
const newCount = queueCounts.get(id) - 1;
if (newCount <= 0) {
queueCounts.delete(id);
locks.delete(id);
} else {
queueCounts.set(id, newCount);
}
}|
|
||
| try { | ||
| const start = Date.now(); | ||
| const result = await fn(); |
There was a problem hiding this comment.
Check if fn() hangs or throws an error, it could block the queue for that id.
If such scenarios happen , add a timeout wrapper or error isolation, something like this,
const timeout = (ms) => new Promise((_, reject) => setTimeout(() => reject(new Error("Timeout")), ms));
const result = await Promise.race([fn(), timeout(10000)]); // 10s timeout
Details of changes: