Create New Index For BHA

[AmitPhulera]

Product Description

This PR is cherrypicked from #34602 and commits were amended to be more chronological and were merged wherever necessary.
This PR extracts out the index creation part and adapter boilerplate from the mentioned PR.

I have updated the migrations to be run only on SaaS environments.

Feature Flag

NA

Safety Assurance

Tested migration locally by applying it and unapplying it, it worked fine.

Safety story

Does not alter anything in HQ just adds a new index that would be created when this will be deployed.

Automated test coverage

NA

QA Plan

NA

Migrations

• The migrations in this code can be safely applied first independently of the code

Rollback instructions

The migrations in the PR should be rolled back before this PR is reverted.

Labels & Review

• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[AmitPhulera]

An unrelated Bootstrap 5 test is failing on the PR.

[gherceg]

Thanks for the simple commits! (Sounds like the failing test will need to be resolve on a separate PR)

[gherceg]

Thank you!

[esoergel]

I really like the "sub indices" framing you added below. If the parent can access the relationship in reverse, that might also be a nice way to fork migrations and writes, like the main index method could do something like this:

        doc_id, source = self.from_python(doc)
        self._verify_doc_id(doc_id)
        self._verify_doc_source(source)
        self._index(doc_id, source, refresh)
        for sub_index in self.get_sub_indices():
            sub_index._index(doc_id, source, refresh)

[AmitPhulera]

In my opinion, it would have made sense if we would have to always multiplex the writes. The writes are conditional based on domain and if the multiplex settings are configured. So it would look something like -

        self._index(doc_id, source, refresh)
        for sub_index in self.get_sub_indices():
            if should_multiplex(source['domain']):
                sub_index._index(doc_id, source, refresh)
            

Thoughts?

[esoergel]

Yeah I like something like that - I was thinking the sub index should be the thing that decides whether to write or reject a doc, so that could be built in to its implementation of _index, or it could provide a is_relevant method or some such.

[AmitPhulera]

That would make sense. Another thought, the current implementation can utilize bulk method to write in both the indices.
The sub_indices approach would require us to make multiple queries which should be fine I think but definitely would be cleaner and more segregated.

[esoergel]

Ah that's a good point - I do think bulk writes make sense too. That could be done by treating sub indices as contributors, something like:

bulk_payload = self._get_payload(doc_id, source)
for index in sub indices:
    payload = index._get_payload(doc_id, source) 
    if payload:
        bulk_payload.extend(payload)

[esoergel]

I could've sworn India was also a SaaS environment, but it looks like this is actually just prod + staging

[AmitPhulera]

Yeah. I also used to think that way until I stumbled upon it 😅

[AmitPhulera]

@esoergel @gherceg Requesting a re-review, merged against master to fix the failing test.

[gherceg]

Looks like there was a mismatch between using parent_index_name on the BHA adapter and parent_index_cname on the base adapter. I updated to use parent_index_name since the comment on the base adapter explains it is the cname.

[AmitPhulera]

Looks like there was a mismatch between using parent_index_name on the BHA adapter and parent_index_cname on the base adapter. I updated to use parent_index_name since the comment on the base adapter explains it is the cname.

Thanks @gherceg. I wanted to go with cname instead of index name because index name will change when we update index multiplexing settings and cname would remain same. I have updated the variable name to reflect the same.

[sravfeyn]

LGTM

[AmitPhulera]

This has been rolled out on staging without any issues. Just to keep in mind to enable cluster routing before rolling this out on prod.

[mkangia]

All follow up steps were done on this as noted here,

1. config updated
2. services restarted
3. backfilled data in next indices