[HMA] Add explicit connection close for signal ops and clear temp refs#1968
Conversation
|
Have we figured out a way to collect evidence on what the source of memory is for longer running processes? At least internally we have some tools that allow us to inspect the memory usage. There are other things that we could do to flail at the problem such as rewriting index build to try and make it incremental, and only rebuild once the delta starts to get too high. In practice I think this reduces the need to rebuild the index at all to zero for most workloads. |
| built_index = index_cls.build(signal_list) | ||
|
|
||
| # explicitly free the signal list before returning | ||
| signal_list.clear() |
There was a problem hiding this comment.
I'm very suspicious this does anything, because it requires that the local variable is not properly garbage collected when it leaves scope, which stretches my understanding about how refcounting works.
Do we have any evidence that this is needed?
Random googling:
| raw_conn.commit() | ||
| finally: | ||
| # explicitly close the raw connection to free memory | ||
| raw_conn.close() |
There was a problem hiding this comment.
This might work too: https://docs.python.org/3/library/contextlib.html#contextlib.closing which lets you use a with statement.
Reading https://docs.sqlalchemy.org/en/21/core/connections.html#working-with-the-dbapi-cursor-directly I think this is very likely missing the close
Summary
Attempts to fix memory leaks in the index lifecycle that cause worker RSS to climb steadily with each index refresh (see #1813).
Close
raw_conninload_signal_index()andcommit_signal_index()previously these database connections were never returned to the pool, leaking on every refresh cycle.Clear
built_indexreference andsignal_listbefore callingtrim_process_memory()sogc.collect()can actually reclaim them without waiting for python to clear reference.