import: Parallel fast-import processes

[craigds]

Description

I noticed we were pegging a CPU core for a git-fast-import process for many hours during a large import (tens of millions of features).

This feeds features from the existing single import source connection
to multiple (default=4) git-fast-import processes.

Performance

The speedup is much better than I expected. I'm suspicious actually:

--num-processes=1

  10,000 features... @317.6s

I killed it at this point; I don't have all day.

--num-processes=4 (the default)

  10,000 features... @9.8s
...
  50,000 features... @196.6s

This doesn't quite make sense to me - that's over 30x faster with only 4x more processes! But it is repeatable.

My sample repo easily achieves 100% saturation of four cores, with sno still basically idling:

I wonder if this unexpectedly good speedup is something to do with disk write caching. Since each process is writing to a subset of the trees, rather than writing to any random tree, perhaps the kernel (or git-fast-import itself) has some kind of per-process write cache which is now performing much better than previously. I don't have any other ideas at the moment (but fast is good! I'll take it)

--num-processes=8, let's see how far we can take this

  10,000 features... @2.1s
...
  50,000 features... @46.0s

or 3x as fast as --num-processes=4 and 66x as fast as --num-processes=1.

--num-processes=16

  10,000 features... @0.8s
...
  50,000 features... @11.2s
...
  161,000 features... @38.5s

--num-processes=26

I only have 26 cores available right now so this is where I'll stop:

  10,000 features... @0.9s
...
  50,000 features... @4.9s
...
  161,000 features... @16.9s

... 352x faster than --num-processes=1 🤨

Amazingly, I can still saturate 26 cores:

Checklist:

• Have you reviewed your own change?
• Have you included test(s)?
• Have you updated the changelog?

[olsen232]

Instead of referring directly to Dataset2, you should be using the constant SUPPORTED_DATASET_CLASS...

... for some reason. It maybe doesn't matter too much

[rcoup]

Be interesting to see if this is actually much quicker? — should try it before we merge anything.

How much CPU was python cf git-fast-import using, and where were the profiled hotspots in Python?

[craigds]

The repos appear to be legit; I get the same exact feature tree with --num-processes=4 and --num-processes=26

update: I've actually checked a bunch of features against the source db; it really does look legit 👍

[craigds]

How much CPU was python cf git-fast-import using, and where were the profiled hotspots in Python?

each git-fast-import always saturates a core; I haven't managed to get python past about ~25% of a core even with --num-processes=26

[rcoup]

LGTM 🚀

[rcoup]

this is a global path, right? How does this work with multiple processes? So each process deletes everything independently, which clears the trees?

[craigds]

correct - but they work on different refs and thus different root trees. There's no conflict here; each starts by deleting a ton of stuff and then reimports separate non-overlapping stuff.

e.g process 0 will import tree ab, process 1 will import tree ac, process 0 will import tree ad, etc.

Because process 1 started by completely deleting tree ab, that tree won't conflict with process 0's tree later when we merge them.

[rcoup]

wonders if there's any benefit to deleting at all? Rather than just starting from empty trees and then combining them at the end?

[olsen232]

Oh yeah this is probably something to do with enum ReplaceExisting:

1. DONT_REPLACE importing a brand new dataset you don't currently have (all existing data is retained)
2. GIVEN importing a dataset over the top of a single existing dataset (retaining all other datasets)
3. ALL importing a dataset over the top of the entire repo (no data is retained)

So another way to look at it, if your repo has N datasets, DONT_REPLACE starts the import from a base of N, GIVEN starts from a base of N-1, and ALL starts from a base of 0.

Which type of ReplaceExisting do you use for mirroring? If it's ALL, then this delete code won't be run anyway for mirroring, so it doesn't make much difference to us... you'd just be cleaning this up for a (hypothetical) user who is using mode GIVEN, which is the only mode that needs to start from somewhere fancy where things need deleting to get to N-1 - starting from N (where we are now) or from 0 (the empty tree) is trivial. Consider switching to ALL if you are not, since it should be slightly cheaper...

But Rob is right - perhaps with your new found tree merging skills, we could do some sanity checks, and then we would always do a ReplaceExisting.ALL to import this dataset, merge all its trees into a single tree... and then, merge that tree in with the rest of the repo for a ReplaceExisting.GIVEN or DONT_REPLACE. It might even make the code a bit simpler

[craigds]

But --replace-ids is a list of IDs to reimport. It doesn't cover the whole dataset. The rest of the dataset should be left untouched.

[rcoup]

does it need deleting? Adding a new blob doesn't just replace the existing one?

[craigds]

The IDs in --replace-ids might no longer exist in the import source, and if that's the case then we have to delete them from the dataset.

Then immediately after this, they're fetched from the import source and reimported if they do exist