On 3/19/25 20:53, Tomas Vondra wrote:
> On 3/19/25 18:11, Tomas Vondra wrote:
>> On 3/19/25 14:43, Tomas Vondra wrote:
>>>
>>> ...
>>>
>>> I'll get this fixed shortly. Unfortunately, this means the "bloom"
>>> filters may be broken - not just those built in parallel, the union
>>> method can be triggered due to concurrent activity too.
>>>
>>
>> Here's a more complete version of the fix, along with a proper commit
>> message, etc.
>>
>> While working on this, I realized there's a second (less severe issue,
>> in that it fails to free the decompressed filters. I believe this would
>> be mostly harmless before parallel builds, because we'd merge only one
>> summary at a time, I think. With parallel builds it may be much worse,
>> but I'm yet to try how bad.
>>
>> FWIW I think the minmax-multi has a similar memory leak.
>>
>
> After looking at this a bit closer, I realized the memory leak is much
> less severe than I thought. The union_tuples() function does all the
> work in a local memory context, so it's not the case we'd keep the
> decompressed filters until the very end of the build.
>
> I plan to simplify the fix a bit by not freeing filter_b.
>
Pushed. I ended up leaving the pfree() filter_b in, even if it's not
strictly necessary. The memory should be released after merging each
range, but for large BRIN ranges and accurate bloom filters it might
still be quite a bit of memory. So let's be tidy.
Thanks for the report!
--
Tomas Vondra
