I wouldn't call this complex. It is much easier to roll it back, so I can work on a proper fix without impacting the ongoing work of others.
The existing Elasticsearch DAOs do not distinguish between document ID and Metron GUID as there was no need to before. So I need to disambiguate those concepts a bit, which is rather subtle. In addition, none of the integration or e2e tests caught the problem because there is a disconnect between the reader and writer side of the house for Elasticsearch. I want to update the tests to ensure this sort of problem is caught. On Tue, Oct 23, 2018 at 3:11 PM Simon Elliston Ball < si...@simonellistonball.com> wrote: > Would it not make more sense to fix the bug on the DAO side, and roll > forward? I suspect what we need to do is add a stage in the update > capability to configure the key field used for update, or worst case have a > pre-query to lookup the internal ID in the relatively rare scenario where > we escalate / modify indexed docs. Seems like a simple new ticket, rather > than a complex roll back and roll forward later. As long as we get the > follow on in before an Apache release we should be fine, no? > > Simon > > On Tue, 23 Oct 2018 at 19:58, Nick Allen <n...@nickallen.org> wrote: > > > Hi Guys - > > > > @rmerriman tracked down some problems that were introduced with my PR > > #1218. Thanks to him for finding this. The change was intended to > improve > > Elasticsearch write performance by allowing Elasticsearch to set its own > > document ID. > > > > The problem is that if you then go to the Alerts UI and escalate an > alert, > > it will create a duplicate alert in the index, rather than updating the > > existing alert. I've been looking at how to fix the problem and the scope > > of the fix is larger than I'd like to handle as a follow-on. There are > > some prerequisites I'd like to tackle before introducing this change. > > > > I am going to revert the change on master, which will introduce an > > additional commit that is an "undo" of the original commit. I will then > > open a separate PR that introduces this new functionality. > > > > https://github.com/apache/metron/pull/1218 > > > > Thanks > > > > > -- > -- > simon elliston ball > @sireb >
