Rollover api for main indices #1309

pavolloffay · 2019-01-31T14:57:58Z

Resolves #1242 #1308

Other significant changes:

moved mappings to separate files - it's important if we want to consume them in other components e.g. rollover script needs mappings when creating index.
json files are compiled to go as we do with static files in query service
marked --es.use-aliases as experimental

mark aliases as experimental

cc @jaegertracing/elasticsearch

Makefile

pavolloffay · 2019-02-01T08:58:39Z

plugin/storage/es/esRollover.py

        print('ACTION ... one of:')
        print('\tinit - creates archive index and aliases')
        print('\trollover - rollover to new write index')
        print('\tlookback - removes old indices from read alias')
        print('HOSTNAME ... specifies which ElasticSearch hosts to search and delete indices from.')
        print('INDEX_PREFIX ... specifies index prefix.')
+        print('init configuration:')
+        print('\tSHARDS ...  the number of shards per index in ElasticSearch (default 5)')


I should remove this, defaults are in the first line

objectiser · 2019-02-04T09:45:32Z

plugin/storage/es/esRollover.py

    if action == 'init':
-        index = prefix + ARCHIVE_INDEX + '-000001'
+        if create_template:


Is the create_template parameter necessary, couldn't you just check for init action and create the index template for the supplied index_to_rollover?

See how the func is called, it would create templates two times in some cases.

I can only see the three calls to the function - each with different index - so each call could pass the mapping file name. However the first call, for span-archive doesn't seem to create the index template - is that correct?

The template should be created for all indices. I forgot to add the param to archive branch.

This still needs to be done? Still think it would be better to just pass the mapping file name instead of the boolean, so each call to perform_action would just create the one index.

objectiser · 2019-02-07T13:33:17Z

Makefile

+	go get -u github.com/jteeuwen/go-bindata/...
+	go get -u github.com/elazarl/go-bindata-assetfs/...
+
+.PHONE: statik-elasticsearch-mappings


Should this be changed to remove statik from the task?

will be changed in a follow pr with the unification of statik and go-bindata

objectiser · 2019-02-07T13:35:48Z

plugin/storage/es/esRollover.py

    if action == 'init':
-        index = prefix + ARCHIVE_INDEX + '-000001'
+        if create_template:


This still needs to be done? Still think it would be better to just pass the mapping file name instead of the boolean, so each call to perform_action would just create the one index.

objectiser · 2019-02-08T10:56:53Z

@pavolloffay Tried using the following with a fresh Elasticsearch db:

docker run test/jaeger-es-rollover init localhost

and got:

Creating index template jaeger-span
Traceback (most recent call last):
  File "/es-rollover/esRollover.py", line 163, in <module>
    main()
  File "/es-rollover/esRollover.py", line 53, in main
    perform_action(action, client, write_alias, read_alias, 'jaeger-span', 'jaeger-span')
  File "/es-rollover/esRollover.py", line 64, in perform_action
    create_index_template(fix_mapping(mapping, shards, replicas), template_name)
  File "/es-rollover/esRollover.py", line 83, in create_index_template
    r = requests.put(sys.argv[2] + '/_template/' + template_name, headers=headers, data=template)
  File "/usr/local/lib/python3.7/site-packages/requests/api.py", line 131, in put
    return request('put', url, data=data, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/requests/api.py", line 60, in request
    return session.request(method=method, url=url, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/requests/sessions.py", line 519, in request
    prep = self.prepare_request(req)
  File "/usr/local/lib/python3.7/site-packages/requests/sessions.py", line 462, in prepare_request
    hooks=merge_hooks(request.hooks, self.hooks),
  File "/usr/local/lib/python3.7/site-packages/requests/models.py", line 313, in prepare
    self.prepare_url(url, params)
  File "/usr/local/lib/python3.7/site-packages/requests/models.py", line 387, in prepare_url
    raise MissingSchema(error)
requests.exceptions.MissingSchema: Invalid URL 'localhost/_template/jaeger-span': No schema supplied. Perhaps you meant http://localhost/_template/jaeger-span?

The usage message says /es-rollover/esRollover.py ACTION HOSTNAME[:PORT] so implies localhost for host should be fine?

pavolloffay · 2019-02-08T11:42:43Z

The usage message says /es-rollover/esRollover.py ACTION HOSTNAME[:PORT] so implies localhost for host should be fine?

localhost if fine but when using docker the localhost of the container is different than the one where ES runs.

objectiser · 2019-02-08T12:09:08Z

Still getting same error even using:

docker run -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" -e "xpack.security.enabled=false" docker.elastic.co/elasticsearch/elasticsearch:5.6.3

docker run --network host test/jaeger-es-rollover init localhost

so looks like it is not to do with localhost, but schema?

objectiser · 2019-02-08T12:23:24Z

The command that worked is:

docker run --network host test/jaeger-es-rollover init http://localhost:9200

so looks like the usage message needs to be updated to change host:[port] to url?

objectiser · 2019-02-08T13:35:28Z

@pavolloffay Tested without index prefix and all worked great. When I then restarted ES and ran:

docker run --network host -e INDEX_PREFIX=test test/jaeger-es-rollover init http://localhost:9200

I got:

$ curl http://localhost:9200/_cat/aliases
test-jaeger-service-read  jaeger-service-000001 - - -
test-jaeger-service-write jaeger-service-000001 - - -
test-jaeger-span-read     jaeger-span-000001    - - -
test-jaeger-span-write    jaeger-span-000001    - - -

$ curl http://localhost:9200/_cat/indices
yellow open .watches                    YiM56sWTRvKYiSqjglbC1Q 1 1  4  0  35.4kb  35.4kb
yellow open jaeger-service-000001       8dkgLAPTQ0-JNL4WDLV4ig 5 1  0  0    810b    810b
yellow open .monitoring-es-6-2019.02.08 y2H-_7sTRf6kSw_BmJAoow 1 1 65 32 251.5kb 251.5kb
yellow open jaeger-span-000001          vdVviIpURbu54oywhCEN1Q 5 1  0  0    810b    810b

Shouldn't the indexes also have the prefix?

objectiser

Some usage text to update - also appears some failure on travis that needs to be investigated. But otherwise have tested the rollover commands and work great.

objectiser · 2019-02-08T14:43:42Z

plugin/storage/es/esRollover.py


 def main():
    if len(sys.argv) != 3:
-        print('USAGE: [INDEX_PREFIX=(default "")] [ARCHIVE=(default false)] [CONDITIONS=(default {})] [UNIT=(default {})] [UNIT_COUNT=(default {})] {} ACTION HOSTNAME[:PORT]'.format(ROLLBACK_CONDITIONS, UNIT, UNIT_COUNT, sys.argv[0]))
+        print('USAGE: [INDEX_PREFIX=(default "")] [ARCHIVE=(default false)] [SHARDS=(default {})] [REPLICAS=(default {})] [CONDITIONS=(default {})] [UNIT=(default {})] [UNIT_COUNT=(default {})] {} ACTION HOSTNAME[:PORT]'.format(SHARDS, REPLICAS, ROLLBACK_CONDITIONS, UNIT, UNIT_COUNT, sys.argv[0]))


Can you update HOSTNAME[:PORT] to be URL, as only seems to work when specifying http://<host>:9200.

objectiser · 2019-02-08T14:43:57Z

plugin/storage/es/esRollover.py


 def main():
    if len(sys.argv) != 3:
-        print('USAGE: [INDEX_PREFIX=(default "")] [ARCHIVE=(default false)] [CONDITIONS=(default {})] [UNIT=(default {})] [UNIT_COUNT=(default {})] {} ACTION HOSTNAME[:PORT]'.format(ROLLBACK_CONDITIONS, UNIT, UNIT_COUNT, sys.argv[0]))
+        print('USAGE: [INDEX_PREFIX=(default "")] [ARCHIVE=(default false)] [SHARDS=(default {})] [REPLICAS=(default {})] [CONDITIONS=(default {})] [UNIT=(default {})] [UNIT_COUNT=(default {})] {} ACTION HOSTNAME[:PORT]'.format(SHARDS, REPLICAS, ROLLBACK_CONDITIONS, UNIT, UNIT_COUNT, sys.argv[0]))
        print('ACTION ... one of:')
        print('\tinit - creates archive index and aliases')
        print('\trollover - rollover to new write index')
        print('\tlookback - removes old indices from read alias')
        print('HOSTNAME ... specifies which ElasticSearch hosts to search and delete indices from.')


As mentioned above - needs to be URL.

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

codecov · 2019-02-12T14:10:39Z

Codecov Report

Merging #1309 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1309   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         162     162           
  Lines        7282    7295   +13     
======================================
+ Hits         7282    7295   +13

Impacted Files	Coverage Δ
plugin/storage/es/options.go	`100% <100%> (ø)`	⬆️
plugin/storage/es/spanstore/writer.go	`100% <100%> (ø)`	⬆️
plugin/storage/es/factory.go	`100% <100%> (ø)`	⬆️
plugin/storage/es/spanstore/reader.go	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 307c62b...5599108. Read the comment docs.

* Add support for rollover to main indices Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix mapping Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Add prefix Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix test-ci Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix mapping in ES itest Signed-off-by: Pavol Loffay <ploffay@redhat.com> Signed-off-by: Iori Yoneji <fivo.11235813@gmail.com>

* Add support for rollover to main indices Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix mapping Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Add prefix Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix test-ci Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Fix mapping in ES itest Signed-off-by: Pavol Loffay <ploffay@redhat.com>

pavolloffay requested review from black-adder, jpkrohling, objectiser, tiffon, vprithvi and yurishkuro as code owners January 31, 2019 14:57

ghost assigned pavolloffay Jan 31, 2019

ghost added the review label Jan 31, 2019

yurishkuro reviewed Jan 31, 2019

View reviewed changes

Makefile Show resolved Hide resolved

pavolloffay commented Feb 1, 2019

View reviewed changes

objectiser reviewed Feb 4, 2019

View reviewed changes

objectiser reviewed Feb 7, 2019

View reviewed changes

pavolloffay force-pushed the rollover-api-for-main-indices branch from 702541f to 66fbd65 Compare February 8, 2019 08:27

pavolloffay mentioned this pull request Feb 8, 2019

Release 1.10 #1321

Closed

4 tasks

objectiser approved these changes Feb 8, 2019

View reviewed changes

pavolloffay added 3 commits February 12, 2019 14:06

Add support for rollover to main indices

56a1cb7

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

Fix mapping

0bed2ea

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

Add prefix

f6e7c58

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

pavolloffay force-pushed the rollover-api-for-main-indices branch from 9810b7b to f6e7c58 Compare February 12, 2019 13:07

pavolloffay added the storage/elasticsearch label Feb 12, 2019

pavolloffay added 2 commits February 12, 2019 14:41

Fix test-ci

11f2dec

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

Fix mapping in ES itest

5599108

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

pavolloffay merged commit 80b3dbb into jaegertracing:master Feb 12, 2019

ghost removed the review label Feb 12, 2019

pavolloffay mentioned this pull request Feb 15, 2019

Rollover script script does not pass index mappings #1308

Closed

frittentheke mentioned this pull request Mar 25, 2020

Add support for ElasticSearch alias to read spans from (Resolves #68) jaegertracing/spark-dependencies#86

Merged

frittentheke mentioned this pull request Apr 2, 2020

Move ES dependencies index mapping to JSON template file #2149

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollover api for main indices #1309

Rollover api for main indices #1309

pavolloffay commented Jan 31, 2019 •

edited

Loading

pavolloffay Feb 1, 2019

objectiser Feb 4, 2019

pavolloffay Feb 4, 2019

objectiser Feb 4, 2019

pavolloffay Feb 5, 2019

objectiser Feb 7, 2019

objectiser Feb 7, 2019

pavolloffay Feb 7, 2019

objectiser Feb 7, 2019

objectiser commented Feb 8, 2019

pavolloffay commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser left a comment

objectiser Feb 8, 2019

objectiser Feb 8, 2019

codecov bot commented Feb 12, 2019 •

edited

Loading

Rollover api for main indices #1309

Rollover api for main indices #1309

Conversation

pavolloffay commented Jan 31, 2019 • 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

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

objectiser commented Feb 8, 2019

pavolloffay commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser commented Feb 8, 2019

objectiser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 12, 2019 • edited Loading

Codecov Report

pavolloffay commented Jan 31, 2019 •

edited

Loading

codecov bot commented Feb 12, 2019 •

edited

Loading