ES leak partial fix (v1) #6339
ES leak partial fix (v1) #6339
Conversation
Signed-off-by: Manik2708 <[email protected]>
|
/Don't merge |
Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik2708 <[email protected]>
|
https://github.com/jaegertracing/jaeger/actions/runs/12275617395/job/34263203812?pr=6339#step:9:21169 4 goroutines are leaking and I have verified that they are from v8 client not from anywhere else. The reason is, there is no way to close this v8 client, closing the http transport can close some of the go-routines but there are most probably some transports which are started by client which remain in memory as a cache for reusability. Please see this: https://discuss.elastic.co/t/how-to-close-the-connection-using-go-client/350123/2. @yurishkuro |
Signed-off-by: Manik2708 <[email protected]>
|
I have applied the changes you said, but it is leading to more go leakage, earlier only 3-4 go leakages were there but it got increased significantly when same transport was used in both the clients. |
| @@ -144,10 +149,11 @@ func (s *ESStorageIntegration) initSpanstore(t *testing.T, allTagsAsFields bool) | |||
| require.NoError(t, err) | |||
| } | |||
|
|
|||
| func healthCheck() error { | |||
| func healthCheck(t *testing.T) error { | |||
| c := getESHttpClient(t) | |||
There was a problem hiding this comment.
maybe another alternative is to ensure we're only using a single http client for a test, i.e. either initialize it upfront or perhaps make this getESHttpClient function lazily initialize it and schedule a clean-up. I would prefer upfront init because with lazy it's hard to predict the scope (even though it looks like it would happen very early in each of the public tests, so might be ok, worth a try).
There was a problem hiding this comment.
I have tested the approach of declaring it upfront and its working fine as leakages are again decreased to 4
Signed-off-by: Manik2708 <[email protected]>
Which problem is this PR solving?
Description of the changes
elasticsearch_testreducing the number of leaks from 50 to less than 10. Unfortunately we can't fix all the leakages because there is no way to close the v8 client of ES leading to some unclosed http transports. If the changes looks good then we can merge without embeddinggoleak(as it will lead to failure of test)How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test