Make sure adding items to the tracking table for new indexes is scalable

I agree.

What we discussed was probably some setting on the index (or some note somewhere else) that this startTracking() still needs to happen, and checking for indexes with that during cron. Also, when doing the operation (creating or enabling) via the UI, we can check for that flag right away, too, and execute a batch if it is present. (In cron, I don't know how we'd handle it – a cron queue?)

One thing we could also do is have some internal configuration setting for the batch size for this operation (and maybe those other tracker operations that chunk the IDs they get) and just skip the whole thing if the amount of items is smaller than that number – however, this would necessitate having a getAllItemsCount() method on the datasource, which seems a bit crooked. Especially since it will be hard to almost impossible to determine once we have translation support.

That translation support, as it happens, will also make computation harder and the item count larger (the latter only for multilingual sites) for this operation, so this might become a problem for smaller sites then, too. Moreover, it means that it probably won't be possible to say "Give me the next 1000 IDs", but we'd probably have to leave paging to the datasource and just provide the "current page" as a parameter. (Because it can easily load 1000 nodes, but those might then become 2000 or 3000 search items, if all nodes are available in two or three languages.)

The additional of the languages and the need to load the entity makes this a critical task IMHO, doing a loadMultiple() made this like 100 times slower ;)

I've been doing some quick tests with 170k imported nodes and the old approach that just needs the ID's is a bit slow but still works, but the "get the languages" stuff definitely doesn't :)

Another question I guess is if we can optimize it for sites that aren't multilingual (\Drupal::languageManager()->isMultilingual()), those then wouldn't need to care about loading/languages at all?

Another option would be to only support translations with content_translation being enabled (without it, there's at least no UI to properly manage them), then we could possibly rely on the metadata that it stores, although that will possibly move into the entity too, which then wouldn't be queryable anymore (in the way we need it). So might not help us much in the end.

Another note: If we'd optimize for non-multilingual sites, then that might require a re-index when you make it so, but that's not something that you do every other day :) And maybe not even that, if you just hardcode $id:LanguageInterface::LANGUAGE_NOT_SPECIFIED then or something.

Agreed, to all points. I think if the site isn't multilingual, we should just add the "und" suffix for all entities, should be good enough.
And the paging is definitely something we need before we can release the module.

note that with https://drupal.org/node/1966436, the suffix should probably be the site default language and not und.

One thing I noticed: exactly the same problem can appear when changing the enabled bundles for a datasource: if a bundle with enough items is affected, this could easily bring down a site. However, if we fix the basic startTracking(), there would at least be the workaround of temporarily disabling the index, making the change, then re-enabling it.
Looks still a lot harder than the original issue, though, I'm not sure how we could manage to solve this cleanly, without introducing a lot of extra dependency. On the other hand, a datasource config change leading to items being added/removed sounds like a pretty common use case, so maybe having some API in place for that would make sense?
In any case, we should keep this on our radar, too.

note that with https://drupal.org/node/1966436, the suffix should probably be the site default language and not und.

Seems like core just took care of that itself.

Now working on this, but it's really quite a challenge. As long as we can execute a batch, it should be fine (I don't think there's a way to create an index or change settings/datasources with Drush, so we don't have to worry about that either), but if it's not possible for some reason then I'm really not sure how to do this.
Does someone know how that's in Drupal 8, whether batches will still work automatically only in forms? And at what times couldn't a batch be executed, what would be a realistic scenario for that?
Any ideas how to solve this problem?

Anyways, for now, when I have time to work on this at all, I'll just implement the batch route first and then we can try to solve the remaining scenarios.

I've been working on similar problems in simplenews but it's a bit different there.

One thought I had is that something would have been a method like this: initializeTracker($datasource_id, $page = NULL), possibly on the index?

NULL means the function would internally still use a pager on the datasource, but it would loop itself through 0 to N until done.
If you pass in an integer, it just processes that page.

Drush can enable/disable indexes I think? But that shouldn't be a problem, see https://www.drupal.org/node/873132.

One interesting challenge is config sync, just like field deletion, we need to trigger the tracker initialization there as part of a config sync. There should be a way to do this, I'll ask @alexpott.

What we need to change is to automatically calling startTracking() (that method needs to go I guess) when necessary, instead just flag an index that needs to start tracking.

Drush can enable/disable indexes I think? But that shouldn't be a problem, see https://www.drupal.org/node/873132.

Oh, you're right, forgot about that. Thanks!
But yes, we're already using a Drush batch when indexing (I think), so I knew this wouldn't be a (large) problem either way.

One interesting challenge is config sync, just like field deletion, we need to trigger the tracker initialization there as part of a config sync.

Doesn't that call the appropriate insert/change/delete hooks and methods already? I'd hope it would, otherwise I'd rather say that's a large bug/deficiency in CMI itself.
Or are you just talking about then setting the batch in this context and be able to run it somehow?

What we need to change is to automatically calling startTracking() (that method needs to go I guess) when necessary, instead just flag an index that needs to start tracking.

Probably something like that, yes.

Yes, sure, config sync calls the hook (but you should always consider the sync status, because syncing and creating new entities often needs you to act differently, you should never change/create other configuration during a sync for example.

Yes, what I meant is automatically extending the batch process to do start the tracking as well. You can use hook_config_import_steps_alter() for this, see field_config_import_steps_alter() for an example.

What you want to do is check if a index is changed or created and then add a step to do the tracking. Maybe that can be a separate issue, but I'm not sure what happens we don't immediately do the tracking, how that will then behave and how you would start it manually. If just re-saving the form would work, that might be an acceptable workaround and we can deal with config sync in a follow-up?

A first try of how it might work. Untested, yet, and sadly I'm now off to a vacation until DrupalCon so can't work more on this until then.
But maybe it already works (though it definitely lacks tests), and in any case running the testbot can't hurt.

OK, of course we now need to trigger this each time we add an index.
(Hm, although, for installed config it should actually be automatic, I guess, thanks to hook_config_import_steps_alter()? Then this doesn't seem to be working. Or it's just not working in the test environment – I'll try it out locally tomorrow.)

This one should at least fix the tests.
I didn't yet manage to test the config import integration, the "Single import" form doesn't seem to trigger that?
Anyways, I'll try to test with the "SA DB Defaults" module.

As Sascha told me, only the complete "Config Synchronize" functionality should trigger that hook, neither single import nor a module install. So, the tests are fine this way, and I tested manually and verified that a config synchronization also works smoothly with this. From all I can see, this patch is finished, working great and ready to be committed.

Sascha, did you also want to test/review or should I just commit?

In any case, I created #2574611: Unify our two task systems for dealing with the slightly different situation of enabling a new bundle for an existing datasource (which has, of course, exactly the same problem).

Feel free to commit this. I'll test asap but I will probably need to update my real sites first before I can test and apply this patch.

OK, thanks for the feedback, and for the help along the way!
Committed.
If you find some problem, probably best just open new issues for those, then.

… and I already have the first one: #2574881: Fix tracking race conditions by making tracker ignore duplicate inserts.