Batch insert post values and other migrations to speed up post import #3453
Conversation
|
It's worth noting that this and bits of #3439 could easily be applied to csv imports and make them much much faster. |
| @@ -26,7 +26,14 @@ class ImportMappingRepository /*extends EloquentRepository*/ implements ImportMa | |||
|
|
|||
| public function create(ImportMapping $model) : int | |||
There was a problem hiding this comment.
From my profiling, it looks a bit like adding the cache-on-write actually made this slower not faster, but I need to find a larger dataset to test on
There was a problem hiding this comment.
That's interesting we should definitely check to see why that might be
| return $tag['id']; | ||
| } | ||
|
|
||
| if (is_numeric($tag)) { |
There was a problem hiding this comment.
This makes a new assumptions that a numeric tag is always the ID. I did consider dropping this parsing entirely, but I think we might be relying on it for CSV imports to map tag names in the CSV to an ID.
There was a problem hiding this comment.
We are definitely relying on it so it needs to remain sadly, it's actually hugely useful in relation to tagging.
There was a problem hiding this comment.
Do you think it needed to remain as it was? or is this partial refactor (assuming numeric = tag id) ok?
There was a problem hiding this comment.
For the purposes of the importer I could pass tags as ['id' => 1] style and that'd be completely safe to skip the lookup on
There was a problem hiding this comment.
No no I think this is fine
| if ($entity->values) { | ||
| $this->updatePostValues($id, $entity->values); | ||
| $postsById = collect($newPostIds)->combine($collection); | ||
|
|
There was a problem hiding this comment.
If this doesn't make sense, I can try to make it more readable. It's an ugly transformation that might be clearer with a foreach instead of all the map/flatten/groupby stuff
There was a problem hiding this comment.
I'm ok with this style but can you expand the comment to be more like:
"reformat set of Posts as...group by 'key' bulk insert attribitues then return X..."
rjmackay
force-pushed
the
migration/batch-insert-post-values
branch
from
7897634
to
313836f
Compare
rjmackay
changed the base branch from
migration/import-incidents
to
migration-tool
- Add createManyValues to UpdatePostValueRepository interface - Implement createManyValues for various value repos - Refactor PostRepository->createMany() to aggregate values and then send them to repo in bulk
…tag/id when the tag is numeric
rjmackay
force-pushed
the
migration/batch-insert-post-values
branch
from
313836f
to
c020099
Compare
This pull request makes the following changes:
Test checklist:
Ping @ushahidi/platform