Importer - Increase size of queue batches #24151
Conversation
|
(Standard links)
|
|
this is fine - I probably would have waited on @andyburnsco testing - but if you want to change in 5.52 that's OK. I have been digging into a performance issue that impacts imports but it was unrelated to this |
|
Yeah, it would be nice to hear back how that went. But he did an emoji on my '100' suggestion... Maybe we go for the safer (Assuming #24152 also passes in a few minutes, I think we could just edit this one to |
I tried 250 and 500 batchSize, the performance did not noticeably change, which is different than my experience with API CSV Importer (it made a difference there). 1 to 1.5s per contact import with several custom fields. 50 or 100 hardcoded is reasonable given normal max execution parameters until this can be a setting on the site and/or per import job. However, I did hit concerning errors and one that broke the db and civi requiring db restore, but could not find a debug and backtrace for it, will test again today. It was a vague On an import that broke the db, it imported 1830 records consecutively (status column = IMPORTED) and then ERROR for the rest with vague @eileenmcnaughton what issues or PR's are the performance related ones? We may be able to pitch in some funding. Background processing, multiple jobs / threads are all priorities for us. |
|
@andyburnsco the biggest part of the performance issue in my testing turned out to be a cache miss - #24156 - we use a Redis cache and that seems to be slower than I might have expected - so I'm not sure if you will get the same improvement with that fix & some related fixes that we did (I'm also going to check in with our sysadmins on the speed of the Redis cache which seems to be a confounding factor). |
|
Please create a gitlab with whatever you can find on the |
|
Seems like this merge did not take. Just noting a small batch queue creates a delay between queues (3-4s in my tests) looking at the detailed logging table. It was 5 this PR #24151 made it 100 now it is 5 again... https://github.com/civicrm/civicrm-core/blob/master/CRM/Import/Parser.php#L673 Noting detailed logging off doesn't seem to have that much of an effect, which is surprising. |
|
50 is probably what it should be and is safest for all installs: #24152 |
Overview ---------------------------------------- We increased batch size to 50 in 5.52 but appear to have accidentally re-set it to 5. This was presumably due to code being merged that patched the code with the older value & hadn't picked up the new Before ---------------------------------------- Each batch is 5 records After ---------------------------------------- Each batch is 50 records Technical Details ---------------------------------------- There seems to be consensus this should be at least 50. 100 is also on the table but I went with Andy's comment civicrm#24151 (comment) The issue Andy points out about the 3 second delay between batches seems excessive and worth digging into. There used to be a 30 second delay in the UI just to let people really luxuriate in watching our batch screen - not sure if that is still there?
|
See #24772 |
Overview ---------------------------------------- We increased batch size to 50 in 5.52 but appear to have accidentally re-set it to 5. This was presumably due to code being merged that patched the code with the older value & hadn't picked up the new Before ---------------------------------------- Each batch is 5 records After ---------------------------------------- Each batch is 50 records Technical Details ---------------------------------------- There seems to be consensus this should be at least 50. 100 is also on the table but I went with Andy's comment civicrm#24151 (comment) The issue Andy points out about the 3 second delay between batches seems excessive and worth digging into. There used to be a 30 second delay in the UI just to let people really luxuriate in watching our batch screen - not sure if that is still there?
Overview
Addresses a simple, recent, and significant change in performance.
See #23669 (comment)