"cannot pass more than 100 arguments" error with actor.org_unit_ancestor_setting_batch

Bug #1568195 reported by Jeff Davis on 2016-04-08
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
High
Unassigned

Bug Description

Evergreen 2.10
Postgres 9.1+ (confirmed on 9.4)

A new actor.org_unit_ancestor_setting_batch database function was introduced via bug 1501471. When loading the Library Settings Editor (at least in the XUL client), this function fails with a "ERROR: cannot pass more than 100 arguments to a function" message in the logs. No warning or error appears in the UI, but setting values will not be populated unless you apply a filter so that fewer than 100 settings are displayed.

The maximum number of arguments allowed by a Postgres server is determined at build time by the value of the FUNC_MAX_ARGS parameter, which defaults to 100 -- see the entry for max_function_args on this page:

http://www.postgresql.org/docs/current/static/runtime-config-preset.html

I imagine all packaged versions of PG just use the default value. We probably want to figure out a way to pass the list of org setting names as a single parameter, rather than as an indefinitely large number of separate values.

Galen Charlton (gmc) wrote :

The stored procedure is called from only one place, OpenILS::Application::AppUtils->ou_ancestor_setting_batch_insecure(), so one solution would be to have that caller split up invocations of the stored procedure. It would be a good idea to measure execution time to compare that approach with marshaling the list of OU names into a single string to pass to the stored procedure.

Galen Charlton (gmc) on 2016-04-09
Changed in evergreen:
status: New → Confirmed
milestone: none → 2.10.2
importance: Undecided → High
tags: added: regression
Galen Charlton (gmc) wrote :

A patch implementing my suggested fix is available at the tip of the user/gmcharlt/lp1568195_retrieve_big_ous_batches branch in the working/Evergreen repository.

Mike Rylander (mrylander) wrote :

If we are going to continue to use a stored proc, we should just build a text array parameter. The complication of chunking just doesn't seem worth the trouble.

Galen Charlton (gmc) wrote :

So, I wrote two more variations:

- pass the list of OU names in a TEXT[]
- pass the list of OUs names in a comma-separated TEXT parameter and use string_to_array().

I then did timing tests using srfsh to open-ils.actor.ou_setting.ancestor_default.batch to retrieve values for all 327 OU setting types defined in my test database, and got the following average run times:

split the list into chunks of 90 at a time: 0.49 seconds
use TEXT[]: 0.50 seconds
use TEXT and string_to_array(): 0.51

Galen Charlton (gmc) wrote :

So, there's probably a sweet spot between reducing the number of times that a stored procedure is invoked and reducing the size of strings that Pg has to parse when invoking the stored procedure, but it seems clear that for our purposes, using a TEXT[] is fast enough -- and it also means that we won't be subject to packagers setting low values for FUNC_MAX_ARGS, nor would we have to check the value max_function_args. Consequently, I'll work up a patch that uses TEXT[].

Galen Charlton (gmc) wrote :

A patch is available at the tip of the user/gmcharlt/lp1568195_retrieve_big_ous_batches_pr branch in the working/Evergreen repository:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1568195_retrieve_big_ous_batches_pr

tags: added: pullrequest
Mike Rylander (mrylander) wrote :

Merged to master and rel_2_10 for 2.10.2 inclusion. Thanks, Galen!

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers