support multi-index ES search

[Rizziepit]

No description provided.

[Rizziepit]

@smn @miltontony I'd like your input on this. I'm weighing up the pros and cons of implementing multi-index search vs. using the same index for multiple repos.

Multi-index

• Pro: We can easily search across a collection of workspaces. I can reuse existing elastic-git functionality to make this happen.
• Con: Elasticsearch's scoring operates per index (more info here). I might be mistaken, but I don't think there is a way to rank results from multiple indices given that the scores aren't comparable. Either of you know more?
• Con: Probably a slight performance hit given the small amount of data per repo.

Single-index, multiple repos

• Pro: Search scoring works as intended.
• Con: We can't use the index to identify the repo. So we have to store that information in Elasticsearch. The one repo = one index assumption in elastic-git means I'll have to rewrite some functionality.

[smn]

How are we going to map the list of workspaces to indexes in the underlying elastig-git S() stuff?

[smn]

What if we changed the idea of workspaces to include multiple repositories?

ws = EG.workspace(dir1, dir2, dir3)

Would we use the same index prefix for all content directories or have separate ones for each and then query multiple indexes with S()?

Which, now that I'm reading things again, is what I suppose you're asking too?

[Rizziepit]

@smn I like upgrading a workspace to have multiple content dirs. I'm slightly inclined towards implementing the same index for all content directories. This approach doesn't break scoring. Is there a good reason to put content dirs in separate indices? @miltontony?

[smn]

Are we sure scoring breaks across different indexes?
I like the idea of separate indexes because then we still have the option of querying them separately.

[Rizziepit]

No, I'm not sure. I'm basing this on the fact that scoring can be wonky across shards. You can explicitly use a DFS query to fix it, but I can't find mention of similar functionality across indices. It might be because people tend to put different types in different indices. I'm probably going down a rabbit hole though. Ignore and implement separate indices?

[smn]

Perhaps we could allow both? Always single indexes, optionally a combined index?

[Rizziepit]

+1

[Rizziepit]

@smn @miltontony feedback on what I've done so far would be much appreciated.

So far I've partially converted Workspace and StorageManager to use multiple repos. It adds a lot of complexity, but I don't think it's a good idea to put multi-repo logic elsewhere (e.g. a Unicore frontend site). Thoughts?

[Rizziepit]

New approach for multi-index search:

# indexes = ['repo1-master', 'repo2-master']
[obj] = S(
    Localisation,
    in_=['repos/repo1', 'repos/repo2'])

# indexes = ['tz-repo1-prod-master', 'tz-repo2-prod-master']
[obj] = S(
    Localisation,
    in_=['repos/repo1', 'repos/repo2'],
    index_prefixes=['tz-repo1-prod', 'tz-repo2-prod'])

Queries return ReadOnlyModelMappingType objects which have a to_object method - get_object raises NotImplementedError.

[Rizziepit]

@smn @miltontony ready for review.

I'm getting a failure locally on repeat test runs because TestSearch.workspace1's index isn't deleted after tests. I can't figure out why. Any ideas?

[smn]

@Rizziepit try running that single test with KEEP_REPO=1 py.test unicore -k <name of test> and then inspect what stuff is left lying around in the .repos dir? The KEEP_REPO environment flag is in elasticgit.tests.base.ModelBaseTest

[smn]

I think we should rather raise a NotImplementedError here instead of removing the function entirely.

[smn]

Sorry for the questions overload, I'm probably missing some obvious stuff...

[smn]

👍 on the code.
Could you add some documentation for this?

[Rizziepit]

@smn this could use a re-review. Instead of a single S class, there is now S and SM where SM is explicitly instantiated with model_class and in_. Workspace still uses S so that it can specify the read-write mapping type.

[smn]

👍 from me on the S() -> SM() changes.

[smn]

👍 again