[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 I'm good +1 ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 As far as THIS PR, I think it has the 2 +1s that I was waiting for. As for a long-range fix, I think it's going to happen outside of this PR. Did I miss anything? ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 I expect to address this in the "777" feature branch/ parser effort. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/834 After reading this whole thread I agree with @ottobackwards. If we installed and started all services and THEN installed our parsers separately this whole issue goes away. If a parser's template doesn't exist or isn't installed correctly, it fails and hopefully we know exactly why. That is obviously a major architectural change and won't be solved in this PR. Since we're in a situation where we do include default parsers as part of the installation, I think they should work out of the box and fail fast if there are not installed correctly. Would some of the concerns be alleviated with https://github.com/apache/metron/pull/831? +1 from me either way. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 Where are we at with this? ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 My comment is more 'big picture'. I don't think it nec. applies to how this PR is resolved. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 @ottobackwards I need to think through the use cases and implications more (e.g. what about a non-ES situation?), but I think your points are valid. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 my comment was re: "It appears to me that we cannot add new templates via the current Ambari mechanism without modifying a release/MPack" ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/834 This is why I put the templates with the parsers in 777. Although I did not have the templates installing from there because list discussion was tough, mainly because talking about this in the abstract is hard. The idea was that in the end we would install with the parsers. Possibly after the ambari flow. - we should be able to install metron without installing and configuring any 'demo' system or parsers at all - we should be able to manage and install templates - templates are part of any parser configuration and should be managed and deployed with the parsers ( ie. 'create an deploy an es template is on the list of what you need to do for a production parser so it goes with the parser workflow') - we shouldn't lay down default configurations for the 'demo' parsers - the demo system should be managed outside main code ( ie. the bro.json files configured with the geo enrichments etc should be separate than what is installed by default) - the demo system should be a separate installable option - all the ES/KIBANA/SOLR issues with mpack etc ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 @nickwallen and @cestella you both make some good points here. The upshot is that this part of our architecture is inextricably tied to ES and in reality we should have another abstraction here. It appears to me that we cannot add new templates via the current Ambari mechanism without modifying a release/MPack. It's fine for managed sensors (bro, yaf, snort, other potential future default sensors), but if a user wants to add new templates, you'd have to manage them outside of Ambari. It's clear to me now that the back and forth on the right way to manage the error handling is because we have a code smell that needs some additional work. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 I ran this up in full-dev and index start functions fine and data is written to the indices. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Nick's points are spot-on, I think. I think the core abstraction issue is that we're doing ES-specific things in the indexing section. We probably want to decouple indexing from ES a bit and have a separate component for ES configuration installation. This also might not be something we want to do as part of ambari, but rather the installation of templates might should be done upon parser start time. This might bear rethinking is what I'm saying. That being said, I think I'm still more uncomfortable with a masked failure like we have it now. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Ok, so a couple of things here. I wanted to have a vigorous discussion, because this change has some pros and cons. At the moment, as Nick alluded, we do template install upon index topology start. The problem is that we cannot depend on ES starting prior to the indexing topology due to us not having a hard dependency listed there in the [role_command_order.json](https://github.com/cestella/incubator-metron/blob/e83390a39910903ccba313a7a1b00433bf347058/metron-deployment/packaging/ambari/metron-mpack/src/main/resources/addon-services/METRON/CURRENT/role_command_order.json). The reason why, I believe, is because we don't assume that users are using ES managed via ambari necessarily. All this to say that there may be a failure to start the indexing topology upon startup, which would necessitate a restart of the indexing topology to retry template install. The downside to what we are doing currently, though, is that a warning is hidden in logs that users probably wont' see. They will see green lights and the UI may or may not work (probably not). This will get worse in ES 5, because without those templates, tuples will fail. So, my question to you guys, is there sufficient value in failing fast here given the broader context. I think so, but I want everyone to realize that we may end up in a situation where indexing fails to start becuase of an ordering problem with ES. I will not push this through without discussion, so I'd like at least 2 +1's from committers and a week of discussion before a commit happens. Thoughts? ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 And for the record @anandsubbu did implement this as "fail fast" in his original PR. So I think he is all for this. I was the one who talked him out of failing fast here. I was wary of introducing a hard dependency between Indexing and ES and didn't think that the "Install Index Templates" step would fail often enough for this to be a problem. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 I clearly see the advantages of failing fast here, but I do have a hesitation. I haven't even totally convinced myself of this. Maybe you guys can help me think down this line of thought and make sure it is a non-issue. This seems like we are introducing a hard dependency between Indexing and Elasticsearch. If Elasticsearch is not running, then I cannot start the Indexing topology. What if I am not using ES, but just want to index into HDFS (or Solr, ha)? Or what if I am need to customize my templates? Does this introduce any pain points that we are not anticipating? ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/834 This is good. +1 by inspection. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/834 Ah, yes, sorry, I should be more clear here. Starting the indexing topology should fail if the indexing templates are not installed. ---
[GitHub] metron issue #834: METRON-1306: When index template install fails, we should...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/834 Just a comment on the description of the JIRA, not the code change itself. Index templates can only be installed when starting the Indexing topology, not during install. During install, Elasticsearch is most likely not running yet. That being said, I think the ask here is that starting the indexing topology should fail, if the index templates fail to install. And this is what you have with your PR. ---
