New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Elasticsearch 6 #5905

Closed
dnsmichi opened this Issue Dec 21, 2017 · 25 comments

Comments

Projects
None yet
7 participants
@dnsmichi
Copy link
Member

dnsmichi commented Dec 21, 2017

Tasks

  • Verify newline fixes from #5795
  • Verify HTTP client fixes from #5760
  • Evaluate whether we can read the backend version from the API, or need a configuration attribute for the feature --> no
  • Evaluate how the beats library deals with that in Icingabeat
  • Test-drive the impact of removing _type with multiple values
  • Analyse other breaking changes and ensure smooth 6.x support
  • Ensure that support for 5.x stays intact
  • Documentation

Current Behavior

Extracted from #5795

Looking deeper shows error in elastic log:

>     [2017-12-21T14:45:01,481][DEBUG][o.e.a.b.TransportShardBulkAction] [icinga2-2017.12.21][1] failed to execute bulk item (index) BulkShardRequest [[icinga2-2017.12.21][1]] containing [index {[icinga2-2017.12.21][icinga2.event.notification][3_NReWAB3ViLs-g_8Mj2], source[{"@timestamp":"2017-12-21T14:43:48.864+0100","author":"","check_command":"http","check_result.check_source":"centos6.silber.rb.de","check_result.command":["/usr/lib64/nagios/plugins/check_http","-I","127.0.0.1","-u","/"],"check_result.execution_end":"2017-12-21T14:43:48.864+0100","check_result.execution_start":"2017-12-21T14:43:48.860+0100","check_result.execution_time":0.0038650035858154296875,"check_result.exit_status":2.0,"check_result.latency":0.0009529590606689453125,"check_result.output":"connect to address 127.0.0.1 and port 80: Verbindungsaufbau abgelehnt\nHTTP CRITICAL - Konnte TCP socket nicht öffnen","check_result.schedule_end":"2017-12-21T14:43:48.864+0100","check_result.schedule_start":"2017-12-21T14:43:48.859+0100","check_result.state":2.0,"check_result.vars_after":{"attempt":1.0,"reachable":true,"state":2.0,"state_type":1.0},"check_result.vars_before":{"attempt":1.0,"reachable":true,"state":2.0,"state_type":1.0},"host":"centos6.silber.rb.de","last_hard_state":2.0,"last_state":2.0,"notification_type":"PROBLEM","service":"http","state":2.0,"text":"","timestamp":"2017-12-21T14:43:48.864+0100","type":"icinga2.event.notification","users":["icingaadmin"]}]}]
>     java.lang.IllegalArgumentException: Rejecting mapping update to [icinga2-2017.12.21] as the final mapping would have more than 1 type: [icinga2.event.notification, icinga2.event.checkresult]
>             at org.elasticsearch.index.mapper.MapperService.internalMerge(MapperService.java:494) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.index.mapper.MapperService.internalMerge(MapperService.java:350) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.index.mapper.MapperService.merge(MapperService.java:282) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.metadata.MetaDataMappingService$PutMappingExecutor.applyRequest(MetaDataMappingService.java:311) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.metadata.MetaDataMappingService$PutMappingExecutor.execute(MetaDataMappingService.java:230) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.MasterService.executeTasks(MasterService.java:640) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:270) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.MasterService.runTasks(MasterService.java:195) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.MasterService$Batcher.run(MasterService.java:130) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:150) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:188) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:569) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:247) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:210) ~[elasticsearch-6.0.0.jar:6.0.0]
>             at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_151]
>             at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_151]
>             at java.lang.Thread.run(Thread.java:748) [?:1.8.0_151]

Possible Solution

  • Provide a default template like beats or logstash
@BjoernRobbe

This comment has been minimized.

Copy link

BjoernRobbe commented Dec 21, 2017

@dnsmichi ElasticsearchWriter uses different types which is not allowed in elastic 6.x

Example:

debug/ElasticsearchWriter: Add to fields to message list: ... "type":"icinga2.event.notification"
debug/ElasticsearchWriter: Add to fields to message list: ... "type":"icinga2.event.checkresult"
@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Dec 21, 2017

Hmmm. Icingabeat does the same which we used as reference here. @bobapple any further ideas?

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Dec 21, 2017

@bobapple this seems to affect Icingabeat too.

From #5795 (comment)

java.lang.IllegalArgumentException: Rejecting mapping update to [test-icinga2-2017.12.21] as the final mapping would have more than 1 type: [icinga2.event.checkresult, icinga2.event.statechange]

java.lang.IllegalArgumentException: Rejecting mapping update to [icingabeat-2017.12.21] as the final mapping would have more than 1 type: [icingabeat.event.statechange, icingabeat.event.checkresult]

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Dec 21, 2017

Me thinks, that type is a reserved keyword in 6.x and as such is used for specifying the type of a message. Meaning to say, you cannot use multiple type values anymore, but need to use a different field.

Can someone confirm that?

@lukasertl

This comment has been minimized.

Copy link

lukasertl commented Dec 21, 2017

The breaking change is documented here:

https://www.elastic.co/guide/en/elasticsearch/reference/6.1/removal-of-types.html

I think the problematic statement is in ElasticsearchWriter::Enqueue:

String indexBody = "{ \"index\" : { \"_type\" : \"" + eventType + "\" } }\n";
@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Dec 21, 2017

Thanks. I have zero knowledge about such things, so I really appreciate any form of hint how to properly patch and migrate that.

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Dec 22, 2017

Elastic posted an article yesterday. https://www.elastic.co/blog/kibana-6-removal-of-mapping-types and how they have migrated Kibana.

First, deciding what the new mapping was going to look like. There are a few common alternatives to mapping types:

- Using a single index per type
- Collapsing all properties under a single type
- Creating a custom type field, and nesting all types under a single type.

We chose adding a custom type field and nesting the data under the name of the type. This allowed us to have fields with the same name under different types, which was previously a limitation.

I still don't know what's the best strategy here for the ElasticsearchWriter. Possible options should follow the format of Icingabeat 6 then, if reasonable.

@dnsmichi dnsmichi changed the title Possible problem with multiple mappings in Elasticsearch 6 Breaking change with unsupported multiple mappings in Elasticsearch 6 Dec 22, 2017

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Jan 8, 2018

If we remove the _type field from the index, we should test whether this needs a compatible flag for 5.x setups, e.g. letting the user decide which version of Elasticsearch is used. Or we determine that by ourselves during the initial connection handshake once. DB IDO does that in a similar fashion too.

I'm open for suggestions here, what's your best practice with Elasticsearch 6.x?

@dnsmichi dnsmichi self-assigned this Jan 8, 2018

@friesoft

This comment has been minimized.

Copy link

friesoft commented Jan 12, 2018

@dnsmichi is there a timeline for the 2.9 release already? As far as I understood 2.8.x will stay elasticsearch 5.x only?

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Jan 12, 2018

@friesoft we should first try to find a possible fix for this and then talk about possible release dates. Anything from your side which would help resolve the problem?

@dnsmichi dnsmichi added the bug label Jan 16, 2018

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Jan 18, 2018

As far as I have read, this implies changes to the way the data is stored in Elasticsearch. As such, this isn't a bug, but a feature request to add 6.x support to the elasticsearch feature in v2.9.

If you urgently need this, sponsoring is welcome.

@dnsmichi dnsmichi changed the title Breaking change with unsupported multiple mappings in Elasticsearch 6 Add support for Elasticsearch 6 Jan 18, 2018

@dnsmichi dnsmichi added enhancement help wanted and removed bug labels Jan 18, 2018

@widhalmt

This comment has been minimized.

Copy link
Member

widhalmt commented Jan 30, 2018

Could we have some sort of rudimentary comptibility matrix for the Elasticsearch writer? Checking all versions might be overkill, but we could state somewhere which versions we tested or we know are working.

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Feb 2, 2018

How would such a matrix help if you know that 5.x works, and 6.x includes breaking API changes not supported by the ElasticsearchWriter feature?

The docs tell you that 5.x is required.

@widhalmt

This comment has been minimized.

Copy link
Member

widhalmt commented Feb 2, 2018

@dnsmichi Ok, you got a point. The information from the docs is fine. Sorry for the fuzz.

@dnsmichi dnsmichi assigned Crunsher and unassigned dnsmichi Apr 5, 2018

@wols

This comment has been minimized.

Copy link

wols commented Apr 5, 2018

I have the Unexpected response code 400 #5795 with icinga2-2.8.2-1 and elasticsearch-5.6.8 also!

# curl -v -k http://127.0.0.1:9200
* About to connect() to 127.0.0.1 port 9200 (#0)
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 9200 (#0)
> GET / HTTP/1.1
> User-Agent: curl/7.29.0
> Host: 127.0.0.1:9200
> Accept: */*
> 
< HTTP/1.1 200 OK
< content-type: application/json; charset=UTF-8
< content-length: 331
< 
{
  "name" : "devop1",
  "cluster_name" : "ntpstats-ng.devop1",
  "cluster_uuid" : "QhqWaLiaS9mXkEmtrB8cPA",
  "version" : {
    "number" : "5.6.8",
    "build_hash" : "688ecce",
    "build_date" : "2018-02-16T16:46:30.010Z",
    "build_snapshot" : false,
    "lucene_version" : "6.6.1"
  },
  "tagline" : "You Know, for Search"
}
* Connection #0 to host 127.0.0.1 left intact
[2018-04-05 22:09:29 +0000] debug/ElasticsearchWriter: Timer expired writing 1 data points
[2018-04-05 22:09:29 +0000] notice/ElasticsearchWriter: Connecting to Elasticsearch on host '127.0.0.1' port '9200'.
[2018-04-05 22:09:29 +0000] debug/ElasticsearchWriter: Sending POST request to 'http://127.0.0.1:9200/icinga2-2018.04.05/_bulk'.
[2018-04-05 22:09:29 +0000] debug/HttpRequest: line: HTTP/1.1 400 Bad Request, tokens: 4
[2018-04-05 22:09:29 +0000] warning/ElasticsearchWriter: Unexpected response code 400

The index icinga2-2018.04.05 has never been created before.

@Crunsher

This comment has been minimized.

Copy link
Member

Crunsher commented Apr 6, 2018

@wols Do you happen to have index.mapping.single_type set?

@Crunsher

This comment has been minimized.

Copy link
Member

Crunsher commented Apr 6, 2018

Current observed issue:
There are two ways of getting rid of the multiple type problem. The first is making the types into indices, the second would be crating a custom type mapping. I'm not a big fan of either 😆

Splitting the indices by (icinga) type is what makes most sense to me right now.

@wols

This comment has been minimized.

Copy link

wols commented Apr 6, 2018

My simple setup based on https://www.icinga.com/docs/icinga2/latest/doc/09-object-types/#objecttype-elasticsearchwriter and no, I have not made any index mapping adjustments (elastic/elasticsearch#24961).

My daily work contains a small ES cluster, data-filled by Logstash and monitored with Icinga2.
ElasticsearchWriter is my first test of integration - BUT I see ElasticsearchWriter-5.x, -6.x, -7.x, -8.x dependencies coming.
That's not what I expect from the Icinga2 (core) team. In the meantime I would rather see a plugins-inputs-icinga2 module: a perfect adapter between Icinga2-API, -state, -perf and Logstash. With one blow I would have a working connection to two different ES clusters via plugins-outputs-elasticsearch :-) https://www.elastic.co/guide/en/logstash/current/plugins-outputs-elasticsearch.html#plugins-outputs-elasticsearch-hosts

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Apr 8, 2018

@wols your problem is discussed and fixed in #5795 (comment) - this targets 2.9 which is not yet released. In terms of other integration discussions please join the community channels, this is getting off-topic here.

This issue focusses on getting 6.x support fixed.

Crunsher added a commit that referenced this issue Apr 11, 2018

@Crunsher

This comment has been minimized.

Copy link
Member

Crunsher commented Apr 11, 2018

About reading the version: Yes, the document root has the version in a json dictionary. But at what point would we read it? If during startup it might be that ES is not running yet and periodically checking the version sounds a bit overkill to me.
We are now supporting the most recent versions with the patched Elasticsearchwriter, what's the rationale for version checking?

@Crunsher

This comment has been minimized.

Copy link
Member

Crunsher commented Apr 11, 2018

Analyse other breaking changes and ensure smooth 6.x support

I didn't find anything else in the docs and changed from elastic 6 to 5 and back. Only thing I had to do were configuration changes in kibana.

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Apr 11, 2018

Version handling was just an idea coming from the IDO database land. If we run into different problems with ES 7 we may introduce a configuration option to specify the version. I wouldn't bother with doing handshakes and reading the version from the API on (re)connect, that overly complicates and slows down the feature.

@dnsmichi dnsmichi removed the help wanted label Apr 11, 2018

@Crunsher

This comment has been minimized.

Copy link
Member

Crunsher commented Apr 12, 2018

Aye, let's halt that idea until we need it then.

dnsmichi added a commit that referenced this issue Apr 17, 2018

@dnsmichi

This comment has been minimized.

Copy link
Member

dnsmichi commented Apr 17, 2018

Changes verified working for both 5.x and 6.x without the specific need to configure a version. Since there's changes involved with the index, no backport to support/2.8 possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment