Closed ezra-varady closed 1 year ago
Rebased onto main. Broke out code to check memory use into a helper function in utils.c. I added code to include memory allocated in the current context and it's children in these calculations for versions of postgres greater than 12. Before pg13 there's not a clear way to check how much memory has been allocated by a single context. The call site in the node retriever will get called a lot. It may be worth trying to optimize a bit around this
Looks good! moved to #205. Will change one test, check code coverage and merge from there
This adds checks for the
work_mem
andmaintenance_work_mem
GUC variables. Checks for both are fairly uncommon in the upstream extensions. It seems likemaintenance_work_mem
is checked more often thanwork_mem
this may be because it's greater size makes it less prohibitive to respect. Neither is actually enforced by postgres. I'll list some examples belowmaintenance_work_mem
is referenced more infrequently but its use is pretty straightforward when it is insrc/backend/access/gin/gininsert.c
in the build callback functionmaintenance_work_mem
is checkedsimilarly nbtree checks it in its parallel build functions
src/backend/access/nbtree/nbtsort.c
It is also checked in
src/backend/commands/vacuumparallel.c
. It is never checked in contrib. I think thebloom
in the earlier listing refers to an internal bloomfilter, not the extension. Notably though pgvector does have a checkwork_mem
is also checked very infrequently although within the optimizer/executor there are a number of checks. insrc/backend/access/gin/ginfast.c
it gets checkedit gets checked in
src/backend/access/nbtree/nbtpage.c
as well, albeit in a function that is only called during vacuumsI have however found the following calling pattern in several places including
contrib/tablefunc/tablefunc.c
,contrib/dblink/dblink.c
,contrib/adminpack/adminpack.c
and also pgvector (albeit only in ivfscans)this seems to be a data structure that holds tuples to be returned by a scan. It doesn't account for memory allocated elsewhere though. Overall the lack of enforcement seems to make checking these values somewhat uncommon. I think it makes sense to enforce
maintenance_work_mem
because building an index is relatively infrequent, but maybe enforcing runtime checks forwork_mem
is overkill