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

Reply via email to