Currently Koha's OAI-PMH provider is using LIMIT and OFFSET to return a chunk of records for each request. While it works relatively well for a small to medium database, it gets prohibitely expensive for a large database especially when items are included and there are a lot of them. I believe this can be improved by using biblionumber for choosing the starting point of the chunk. Patch coming up soon.
Created attachment 116184 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Uses next biblionumber instead of large offset in the queries. Test plan: 1. Without the patch, try harvesting a Koha database with `include_items: 1` in the OAI-PMH configuration file pointed to by preference OAI-PMH:ConfFile and take note of performance. For useful metrics the database must be large enough to not fit in InnoDB buffers or OS file cache. 2. Apply the patch. 3. Run tests: prove -v t/db_dependent/OAI 4. Try again the harvesting from step 1 and compare performance with step 1.
Comment on attachment 116184 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Initial patch was bad, needs some fixing.
Comment on attachment 116184 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Oops, the patch is fine (I messed up with Plack).
Comment on attachment 116184 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance I believe there's an even faster way, a patch coming up when done benchmarking and testing.
Curious to see what you come up with here. -- Here's a little look at doing a UNION on nearly 600,000 biblio and deletedbiblio entries. EXPLAIN select * from (select biblionumber from deletedbiblio UNION select biblionumber from biblio) u limit 1; +------+--------------+---------------+-------+---------------+----------+---------+------+--------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+--------------+---------------+-------+---------------+----------+---------+------+--------+-------------+ | 1 | PRIMARY | <derived2> | ALL | NULL | NULL | NULL | NULL | 578038 | | | 2 | DERIVED | deletedbiblio | index | NULL | blbnoidx | 4 | NULL | 4361 | Using index | | 3 | UNION | biblio | index | NULL | blbnoidx | 4 | NULL | 573677 | Using index | | NULL | UNION RESULT | <union2,3> | ALL | NULL | NULL | NULL | NULL | NULL | | +------+--------------+---------------+-------+---------------+----------+---------+------+--------+-------------+ 4 rows in set (0.00 sec) select * from (select biblionumber from deletedbiblio UNION select biblionumber from biblio) u limit 1; +--------------+ | biblionumber | +--------------+ | 3 | +--------------+ 1 row in set (13.39 sec) OR select * from (select biblionumber from deletedbiblio UNION select biblionumber from biblio) u limit 0,50; 50 rows in set (12.60 sec) select biblionumber, (select metadata from biblio_metadata where biblionumber = u.biblionumber) from (select biblionumber from deletedbiblio UNION select biblionumber from biblio) u limit 0,50; 50 rows in set (13.58 sec) -- Of course, there's no index on `timestamp`, so we can't sort that list. However, perhaps if we added a composite `timestamp,biblionumber` index to the biblio and deletedbiblio tables... Still... 13 seconds isn't brilliant.
Other ideas... would be moving the index to a different table or a different system. In theory, there's no reason why we couldn't use Zebra or Elasticsearch for doing OAI. If I recall correctly, I think that DSpace uses Solr for its OAI. But the further away from the database, the less likely it's going to be correct/up-to-date.
Hold on, a new version coming up shortly. Much improved, I believe! :)
I think there's an open bug somewhere to harmonize the biblio and deletebiblio tables as well. That's probably the most optimal plan...
(In reply to Ere Maijala from comment #7) > Hold on, a new version coming up shortly. Much improved, I believe! :) End of the work day for me, mate. Actually, that was an hour ago, but performance always thrills me. Looking forward to the new version!
Created attachment 116309 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Includes the following optimizations: - Use next biblionumber instead of large offset in the queries. - Use unions instead of subqueries - Avoid fetching item timestamps when items are not included. Test plan: 1. Without the patch, try harvesting a Koha database with (and without for good measure) `include_items: 1` in the OAI-PMH configuration file pointed to by preference OAI-PMH:ConfFile and take note of performance. For useful metrics the database must be large enough to not fit in InnoDB buffers or OS file cache. 2. Apply the patch. 3. Run tests: prove -v t/db_dependent/OAI 4. Try again the harvesting from step 1 and compare performance with step 1.
Created attachment 116310 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Includes the following optimizations: - Use next biblionumber instead of large offset in the queries. - Use unions instead of subqueries - Avoid fetching item timestamps when items are not included. Test plan: 1. Without the patch, try harvesting a Koha database with (and without for good measure) `include_items: 1` in the OAI-PMH configuration file pointed to by preference OAI-PMH:ConfFile and take note of performance. For useful metrics the database must be large enough to not fit in InnoDB buffers or OS file cache. 2. Apply the patch. 3. Run tests: prove -v t/db_dependent/OAI 4. Try again the harvesting from step 1 and compare performance with step 1.
This should work pretty well. I just haven't had time yet to test with a large number of items, but I'll try to accomplish that as well (and hope it works...).
Some benchmarking results: My test system is intentionally memory-constrained, items are included and OAI-PMH:MaxCount = 1000. All values are measured from 100 requests starting at offset 190341 when doing a full harvesting without date limits. The database contains 0.97 million biblios and 2.4 million items. Reported times are averages with standard deviation in parentheses. Full duration of oai.pl: 12.98s (0.94s) Biblionumber query: 0.0024s (0.0024s) Fetch+create record: 6.84s (0.51s) Creating response: 4.97s (0.50s) As far as I can see, this represents the full harvesting run pretty well, so it doesn't slow down anymore when getting to higher biblionumbers. As the results indicate, query duration for a set of results is now pretty much meaningless. We spend most of the time collecting the record metadata and creating a DOM for it, and then writing the actual response, which is part of the HTTP::OAI module. So to sum it up: with these changes the biblionumber query is no longer a bottleneck. Previously, it easily took 15 to 20 seconds on my test system.
Adding to the previous results, harvesting of the forementioned records completed in less than 5 hours, and the harvesting speed was pretty much constant.
After reviewing the code again, I'm only now realizing that we're not even trying to do a UNION of biblios and deleted biblios, so the results aren't in date order... Although after reviewing the OAI-PMH spec, it actually explicitly says that date ordering should not be assumed: "The protocol does not define the semantics of incompleteness. Therefore, a harvester should not assume that the members in an incomplete list conform to some selection criteria (e.g., date ordering)." You learn something new every day...
At a glance, I think that your code is probably an improvement. However, looking at Koha::OAI::Server::ListBase makes me wonder if we shouldn't denormalize a bit though. If we had a timestamp in a biblio table that included a timestamp for the last item activity, that would remove the need for a lot of these complex SQL queries. If we fetched all the biblio metadata in our first query, we'd also save overhead. But... both of those would involve more work.
My test plan: 0) Use koha-testing-docker 1) Enable "OAI-PMH" 2) Go to http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 3) Apply patch 4) koha-plack --restart kohadev 5) Go to http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6) Note that the result lists are the same 7) Set OAI-PMH:ConfFile to "/kohadevbox/koha/oai-conf.yml" 8) Create oai-conf.yml* 9) Go to http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml 10) Note that identifiers are same as previous list 11) Note that items (952 fields) are included in metadata * --- format: marcxml: metadataPrefix: marcxml metadataNamespace: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim schema: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd include_items: 1 oai_dc: metadataPrefix: oai_dc metadataNamespace: http://www.openarchives.org/OAI/2.0/oai_dc/ schema: http://www.openarchives.org/OAI/2.0/oai_dc.xsd xsl_file: /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/xslt/MARC21slim2OAIDC.xsl
Created attachment 116345 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Includes the following optimizations: - Use next biblionumber instead of large offset in the queries. - Use unions instead of subqueries - Avoid fetching item timestamps when items are not included. Test plan: 1. Without the patch, try harvesting a Koha database with (and without for good measure) `include_items: 1` in the OAI-PMH configuration file pointed to by preference OAI-PMH:ConfFile and take note of performance. For useful metrics the database must be large enough to not fit in InnoDB buffers or OS file cache. 2. Apply the patch. 3. Run tests: prove -v t/db_dependent/OAI 4. Try again the harvesting from step 1 and compare performance with step 1. Signed-off-by: David Cook <dcook@prosentient.com.au>
As a tester, I'm not really commenting on performance. I'm just confirming that the code doesn't break anything and works as a user would expect. For what it's worth, on a small database, it's quite quick. I don't have a big enough test database on hand at this moment to test the code on. But at a glance it looks like it should be fine.
Thanks for the comments and review! Indeed, things will be easier when deleted records are included in the normal tables, but for now this is quite alright, and there's no need to union them all together. I think there's an advantage with tracking the different timestamps even if it's more complicated. When item data is not included, it wouldn't be useful to harvest biblios as updated when an item changes, since the biblio record would be identical. If you meant that we could have another timestamp that would indicate the latest change for the logical record that OAI-PMH would provide, yeah, that'd work, but trying to track latest item changes in biblios would complicate other functionality and could also have unintended consequences such as increased overhead for batch operations. Also, I'm still optimistic that we can get bug 20447 merged somewhere in the future, and that would add to the complexity. As I see it, the "proper" solution would be to have a publishing process that would run in background to create sets of records for harvesting. With published sets we could handle inclusion of items, deletions etc. in the publishing process, and the OAI-PMH provider would only need to serve the results. However, this would be a whole lot more complicated than what we currently do, and there'd be a fair chance that the publishing process would do a lot of work to create result sets that nobody ever harvests. Additionally, it would make quick (semi-realtime) incremental harvesting impossible.
It's not without wonder, bug as it is, Koha OAI Server has now become a dark mystery to me, a little like Jean Sibelius violin concerto in D minor op 47.
(In reply to Ere Maijala from comment #20) > > I think there's an advantage with tracking the different timestamps even if > it's more complicated. When item data is not included, it wouldn't be useful > to harvest biblios as updated when an item changes, since the biblio record > would be identical. Agreed > If you meant that we could have another timestamp that > would indicate the latest change for the logical record that OAI-PMH would > provide, yeah, that'd work, but trying to track latest item changes in > biblios would complicate other functionality and could also have unintended > consequences such as increased overhead for batch operations. > I meant we'd keep the existing timestamp for the bibliographic data and then add a timestamp to indicate when the bibliographic data + items/holdings data was changed. Then depending on the OAI setting, it would decide which timestamp was relevant. > Also, I'm > still optimistic that we can get bug 20447 merged somewhere in the future, > and that would add to the complexity. > That'll be interesting when that time comes. > As I see it, the "proper" solution would be to have a publishing process > that would run in background to create sets of records for harvesting. With > published sets we could handle inclusion of items, deletions etc. in the > publishing process, and the OAI-PMH provider would only need to serve the > results. However, this would be a whole lot more complicated than what we > currently do, and there'd be a fair chance that the publishing process would > do a lot of work to create result sets that nobody ever harvests. > Additionally, it would make quick (semi-realtime) incremental harvesting > impossible. I don't think that would be feasible. I reckon all we need are some good indexes. But I think this change makes sense for now.
Created attachment 120316 [details] [review] Bug 27584: Refactor OAI-PMH paging to improve performance Includes the following optimizations: - Use next biblionumber instead of large offset in the queries. - Use unions instead of subqueries - Avoid fetching item timestamps when items are not included. Test plan: 1. Without the patch, try harvesting a Koha database with (and without for good measure) `include_items: 1` in the OAI-PMH configuration file pointed to by preference OAI-PMH:ConfFile and take note of performance. For useful metrics the database must be large enough to not fit in InnoDB buffers or OS file cache. 2. Apply the patch. 3. Run tests: prove -v t/db_dependent/OAI 4. Try again the harvesting from step 1 and compare performance with step 1. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 120317 [details] Simple script for testing Simple script for benchmarking and/or checking results With 30k records, none deleted, I got ~3min before patch - ~2:30 with patch With 15k deleted 15k active: ~2min before / ~1:40 after Checking lists of biblionumbers were identical before and after the patches
Nick, thanks for QA and benchmark results. It's good to see that it makes a difference with smaller data sets as well. :)
Pushed to master for 21.05, thanks to everybody involved!
Enhancement not pushed to 20.11.x