DeletedTerms#clear should reset ByteBlockPool #12630
Conversation
mikemccand
left a comment
mikemccand
left a comment
There was a problem hiding this comment.
Good catch @gf2121!
Just to confirm: the previous PR was not released/included in 9.8.0 right? So users are not hitting this memory leak when using the 9.8.0 release.
| public void testDeletedTerms() { | ||
| int iters = atLeast(10); | ||
| String[] fields = new String[] {"a", "b", "c"}; | ||
| BufferedUpdates.DeletedTerms actual = new BufferedUpdates.DeletedTerms(); |
There was a problem hiding this comment.
Does moving the BufferedUpdates up here mean the memory leak is more obvious (i.e. the test now fails with OOME or so, before your fix)?
There was a problem hiding this comment.
Actually I'm moving this to test #clear runs properly, not related to the memory leak issue :)
|
wow good catch. Out of curiosity, how did you catch it? Are you running snapshot Lucene builds in production? |
Yes, the previous PR is not included in 9.8.0 release.
Not really, just come up this accidentally when reading codes :) |
3 participants
Description
This is a bug left by #12573. As we are using a common
ByteBlockPoolacross allBytesRefHash, the map clear won't help release the memory held byByteBlockPooland the pool will allocate new slices when new terms added because the offset is never reset.Sorry for the missing!