Thank you the comment. On Fri, Mar 2, 2018 at 4:18 AM, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > Masahiko Sawada wrote: > >> While reading the code, I realized that the requesting an autovacuum >> work-item could fail in silence if work-item array is full. So the >> users cannot realize that work-item is never performed. >> AutoVacuumRequestWork() seems to behave so from the initial >> implementation but is there any reason of such behavior? It seems to >> me that it can be a problem even now that there is only one kind of >> work-item. Attached patch for fixing it. > > Hmm, yeah. > > I think it would be better to return false to caller, and have them > report the failure. Then they're in a better position to place an > accurate log message -- for instance indicating the relation name rather > than just OID, and specify work item type rather than some weird > integer. (I think the ereport() should definitely be *outside* the > locked section, in any case.)
Agreed. Attached an updated patch. > > I'm inclined to change the API of AutoVacuumRequestWork even in branch > 10. Since this stuff is not nicely extensible (c.f. perform_work_item), > it doesn't seem likely that any outside code is calling it yet. Once we > have hooks to register worker functions and stuff, it'll become more of > a problem. +1 Since this API cannot be execute actually other than brin summarization I think we can change it in branch 10. It's good if autovacuum work-items can be added by extensions and so on. Also I'm inclined to change the autovacuum launcher so that it can launcher new worker for performing work-item or to allow requests to specify the execution timing (before or after vacuum), it's for PG12 item though. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
report_autovac_workitem_request_failure_v2.patch
Description: Binary data
