Comment 4 for bug 1847805

Revision history for this message
Galen Charlton (gmc) wrote :

I've done some initial testing and have some comments:

- using the following query on a Concerto database with a BR3 circulator, I got the following results:

  request open-ils.pcrud open-ils.pcrud.search.au "$SES", {"id":{"!=":null}}, {}

  * the number of records returned were the same both pre- and post- patch
  * pre-patch, it took ~2 seconds on my test system; post-patch, ~3.5 seconds
  * bumping the default cursor window size from 5 to 50 brought it back down to ~2.2 seconds
  * it made no difference to the timing whether or not the query to fetch each row for permissions checks used an interior cursor or not; the additional time it took seemed to be caused strictly by the outer cursor

- I have a major concern about opening the cursor "WITH HOLD". Per https://www.postgresql.org/docs/9.6/sql-declare.html, "In the current implementation, the rows represented by a held cursor are copied into a temporary file or memory area so that they remain available for subsequent transactions." I've verified that creating such cursors does indeed create files in pgsql_tmp for large files; a glitch (or attack) that fired off a number of cstore or pcrud searches of large tables could cause disk usage and/or I/O problems on a database server.

Creating a cursor WITHOUT HOLD inside of a transaction should work without creating temporary files as often as WITH HOLD could, but if that were done, cursors either become something that gets used only within atomic cstore and pcrud calls or making all searches effectively atomic, which would have its own tradeoffs.

- as a minor point, I think it's leaking the cursor_name string