global: allow using an index prefix #145

david-caro · 2018-06-26T18:02:35Z

Now using the setting SEARCH_INDEX_PREFIX you can define a prefix to
use for all your indices.

This partially addresses #129

Signed-off-by: David Caro [email protected]

david-caro · 2018-06-26T18:46:01Z

This seems to be all that is needed. I'm changing the api of one of the utils methods, so this might break someone's code.

slint

I think utils.build_index_name is only used in the module, so changing the signature is probably ok.

One small issue I see is that the prefixing only applies to index names and not aliases... On the other hand, I think that ESonDemand, allowed creation of aliases even without using a prefix... (e.g. I managed to create esod_prefix-records-v1.0.0 and then the records alias which pointed to it worked fine... Maybe the next guy that would try to create a records alias would have issues ¯\_(ツ)_/¯)

slint · 2018-06-26T18:52:06Z

invenio_search/api.py

-        kwargs.setdefault('index', getattr(self.Meta, 'index', None))
+        kwargs.setdefault(
+            'index',
+            current_app.config['SEARCH_INDEX_PREFIX'] + '-' +


Maybe it would be better if the dash (-) was part of the SEARCH_INDEX_PREFIX? I know it's how it's done by ESoD, but it will be hard to change in the future if somebody wants to do it differently

Yep :), I just did that change locally, will push soon.

slint · 2018-06-26T19:01:19Z

invenio_search/api.py

@@ -83,7 +83,11 @@ class Meta:

    def __init__(self, **kwargs):
        """Use Meta to set kwargs defaults."""
-        kwargs.setdefault('index', getattr(self.Meta, 'index', None))
+        kwargs.setdefault(


There are currently two ways to perform queries to different indices using this class:

Sub-classing and overriding Meta.index

Passing index explicitly as a keyword argument

The first case is of course covered... Though the usage of kwargs.setdefault instead of setting the value directly is probably because of the fact that this constructor was written before the need for prefixing index names "magically" existed...

Maybe it would be better to make Meta.index some kind of property which relies on some _index field? @lnielsen WDYT?

Locally I solved it by checking if the kwargs['index'] is prefixed, and if not then adding it (at some point there's also a list passed to it instead a string...)

david-caro · 2018-06-26T20:20:21Z

Btw. this current version does take care of the aliases too \o/

ntarocco · 2018-06-27T15:22:16Z

invenio_search/config.py

@@ -79,3 +79,5 @@
 <https://www.elastic.co/guide/en/elasticsearch/reference/current/
 search-request-min-score.html>`_ for more information.
 """
+
+SEARCH_INDEX_PREFIX = ''


I would have some documentation maybe here, as for the other config variables.

ntarocco · 2018-06-27T15:24:54Z

@slint agree that build_index_name in inveniosoftware organisation is used only here, but maybe in any overlay/other modules this is used, since it is not marked as private.
So we will probably have to release it with a "major" version?

It would be also nice to update the doc and put there an example.

Now using the setting `SEARCH_INDEX_PREFIX` you can define a prefix to use for all your indices. Signed-off-by: David Caro <[email protected]>

lnielsen

Thanks a lot for the PR. It's much appreciated and needed by everyone.

I'm ok with changing the signature of build_index_name as long as it's properly documented in the CHANGES.rst once the new version is released (i.e. 1.1.0).

Regarding aliases, would you mind adding a quick test that demonstrates the prefix for aliases work as well.

Also, what about the ES templates, do they need prefixes as well?

lnielsen · 2018-06-29T11:45:30Z

invenio_search/utils.py

+    :returns: A string with the new index name prefixed in needed.
+    """
+    index_prefix = app.config['SEARCH_INDEX_PREFIX']
+    if index.startswith(index_prefix):


Which cases is this covering? Is it only when SEARCH_INDEX_PREFIX == '' or are there other cases?

Example: If you decided to name your prefix records and your index starts with records I would still expect the final index name to be records-records (might not make much sense, but it's consistent 😄).

That is there because the libs at some point start doing nested calls passing the index string around, so if you have to make sure to prepend the perfix only once. Unfortunately, all the info you get is the index name string, so you can't add random info (like a boolean property prefixed). But, if you want your indices to end up as record-record the prefix should be record-, with the last dash, and that will not really collide (unless you want record-record-something). But I don't know how to prevent that from happening without changing substantially the code.

Got it. Yeah, not much to do about it. Perhaps just add a quick comment in the code, that this line is there due to nested calls.

david-caro · 2018-06-29T12:06:03Z

Regarding aliases, would you mind adding a quick test that demonstrates the prefix for aliases work as well.

I'll try. So far the aliases and indices use the same function to get the names, and that one is the one that does the prefixing (build_index_name).

Also, what about the ES templates, do they need prefixes as well?
I'll check.

dinosk · 2018-08-13T16:11:59Z

After some testing I see that the aliases have the prefix:

alias                     index                                          filter routing.index routing.search
prefix-demo               prefix-demo-bibliographic-bibliographic-v1.0.0 -      -             -
prefix-demo-bibliographic prefix-demo-bibliographic-bibliographic-v1.0.0 -      -             -
prefix-demo               prefix-demo-default-v1.0.0                     -      -             -
prefix-demo               prefix-demo-authorities-authority-v1.0.0       -      -             -
prefix-demo-authorities   prefix-demo-authorities-authority-v1.0.0       -      -             -

But for the templates if I understand correctly we'll need to add it, as after registering a sample one, it's registered in the following way:

...
"prefix-file_download/v6-file-download-v1" : {
    "order" : 0,
    "index_patterns" : [
      "events-stats-file-download-*"
    ],
...

lnielsen · 2018-08-14T12:26:10Z

We've been digging into the prefixing in the current Invenio sprint, and I'd like to discuss if it's worthwhile adding support for prefixing indexes.

Template problem
Currently, as @dinosk mentions, there are some issues with templates. To fix it, we would need to dynamically modify the JSON body sent to Elasticsearch. E.g something like:

{
  "template": "<prefix>-events-stats-record-view-*",
  ...
  "aliases": {
    "<prefix>-events-recordview": {}
  }
}

It's not clear, if it's only template or aliases keys that have to be prefixed or if there are more keys.

Workaround
Currently, it's possible to prefix your indexes by simply adding a further folder:

mappings/v6/records/record-v1.0.0.json -> records-record-v1.0.0.json
mappings/v6/<prefix>/records/record-v1.0.0.json -> <prefix>-records-record-v1.0.0.json

Similar for templates, you can add your prefix inside the template JSON body manually.

CERN Central Elasticsearch Service
One of the use cases for prefixing is to use the central Elasticsearch service at CERN that requires prefixing. That is possible using the workaround above (in use by CERN Search).

Shared Elasticsearch for multiple Invenio instances
In case we do not add support for prefixing, I think the key use case we'll not be able to support is using a shared Elasticsearch instance for several Invenio instances since they would all try to create and manage the indexes (e.g. records-record-v1.0.0.json).

The down-side by using a shared Elasticsearch instance for multiple Invenio instances, is that each instance will have read/write access to all instances unless you add the expensive X-Pack from Elastic with support for authorization and permissions. OTOH you only need to operate one ES instance and thereby save resources.

What to do?
I would like to see prefixing support in Invenio-Search, but I have some doubts about if we can make it robustness enough, and given that there's an existing robust workaround, I'd like to get some inputs on if it's worthwhile the effort. Essentially, is using a shared Elasticsearch instance for multiple Invenio instances such an important use case that it justifies the efforts needed to make the prefixing robust enough?

david-caro · 2018-08-22T07:57:03Z

Something that this seems not to address is having multiple invenio instances on CERN Central Elasticsearch Service, as that would require duplicating all the mappings on two different subdirectories no? (for example, having a 'dev', a 'qa' and a 'prod' instances).

lnielsen · 2018-08-27T13:44:01Z

Correct. The dev, qa and prod is a special case of "multiple Invenio instances on a shared Elasticsearch cluster" which make it useless to use the central Elasticsearch cluster for multiple environments (you could still use it for only one environment).

Another issue is that the support for templates in the central Elasticsearch service. Right now this is handled via a git repo instead of via the API. This would mean e.g. the statistics module won't be usable on the central service (or at least it will be rather cumbersome).

I'm worried that prefixing is not robust enough to properly sandbox two Invenio instances running on the same ES cluster and make sure they don't interfere with each other in any way.

ntarocco · 2019-02-15T17:55:51Z

I am closing this PR because it is outdated, the new PR is here: #156

david-caro force-pushed the allow_prefixing_indices branch 2 times, most recently from 0e45f34 to 054a3e1 Compare June 26, 2018 18:45

slint reviewed Jun 26, 2018

View reviewed changes

slint requested review from lnielsen and ntarocco June 26, 2018 19:10

david-caro force-pushed the allow_prefixing_indices branch from 054a3e1 to 6752274 Compare June 26, 2018 19:45

david-caro mentioned this pull request Jun 26, 2018

global: adapt to prefixed indices inspirehep/inspire-matcher#49

Closed

david-caro force-pushed the allow_prefixing_indices branch from 6752274 to 562bca7 Compare June 26, 2018 20:08

david-caro mentioned this pull request Jun 26, 2018

WIP global: adapt to prefixed indices inspirehep/inspire-next#3517

Closed

david-caro force-pushed the allow_prefixing_indices branch 3 times, most recently from 74f62a9 to 2e8e8bb Compare June 26, 2018 21:59

ntarocco reviewed Jun 27, 2018

View reviewed changes

david-caro force-pushed the allow_prefixing_indices branch from 2e8e8bb to f01f40d Compare June 28, 2018 16:27

global: allow using an index prefix

8a303a2

Now using the setting `SEARCH_INDEX_PREFIX` you can define a prefix to use for all your indices. Signed-off-by: David Caro <[email protected]>

david-caro force-pushed the allow_prefixing_indices branch from f01f40d to 8a303a2 Compare June 29, 2018 08:45

lnielsen reviewed Jun 29, 2018

View reviewed changes

slint mentioned this pull request Jun 29, 2018

ext: simplify mapping/template discovery #146

Open

dinosk self-assigned this Aug 13, 2018

dinosk removed their assignment Aug 14, 2018

ntarocco closed this Feb 15, 2019

ntarocco mentioned this pull request Feb 18, 2019

Allow prefixing indices #156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

global: allow using an index prefix #145

global: allow using an index prefix #145

david-caro commented Jun 26, 2018

david-caro commented Jun 26, 2018

slint left a comment •

edited

Loading

slint Jun 26, 2018

david-caro Jun 26, 2018

slint Jun 26, 2018

david-caro Jun 26, 2018

david-caro commented Jun 26, 2018

ntarocco Jun 27, 2018

ntarocco commented Jun 27, 2018

lnielsen left a comment •

edited

Loading

lnielsen Jun 29, 2018

david-caro Jun 29, 2018

lnielsen Jun 29, 2018

david-caro commented Jun 29, 2018

dinosk commented Aug 13, 2018

lnielsen commented Aug 14, 2018 •

edited

Loading

david-caro commented Aug 22, 2018

lnielsen commented Aug 27, 2018

ntarocco commented Feb 15, 2019

global: allow using an index prefix #145

global: allow using an index prefix #145

Conversation

david-caro commented Jun 26, 2018

david-caro commented Jun 26, 2018

slint left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-caro commented Jun 26, 2018

Choose a reason for hiding this comment

ntarocco commented Jun 27, 2018

lnielsen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-caro commented Jun 29, 2018

dinosk commented Aug 13, 2018

lnielsen commented Aug 14, 2018 • edited Loading

david-caro commented Aug 22, 2018

lnielsen commented Aug 27, 2018

ntarocco commented Feb 15, 2019

slint left a comment •

edited

Loading

lnielsen left a comment •

edited

Loading

lnielsen commented Aug 14, 2018 •

edited

Loading