IndexWriter interface in 1.15

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

IndexWriter interface in 1.15

Yossi Tamari
Hi,

 

I missed it at the time, but I just realized (the hard way) that the
IndexWriter interface was changed in 1.15 in ways that are not backward
compatible.

That means that any custom IndexWriter implementation will no longer
compile, and probably will not run either.

I think this was a mistake (maybe a new interface should have been created,
and the old one deprecated and supported for now, or just the old methods
deprecated without change, and the new methods provided with a default
implementation), but it's too late now.

I still think this is something that should be highlighted in the release
note for 1.15 (meaning at the top, as "breaking changes").

The main changes I encountered:

1. setConf and getConf were removed from the interface (without
deprecation).
2. open was deprecated (that's fine), and its signature was changed
(from JobConf to Configuration), which means it a completely different
function technically, and there is no point in the deprecation.

 

Yossi.

Reply | Threaded
Open this post in threaded view
|

Re: IndexWriter interface in 1.15

lewis john mcgibbney-2
Hi Yossi,

REASON: Upgrade of MapReduce API from legacy to 'new'. This was a breaking
change for sure and a HUGE patch. We did not however factor in the
non-braking aspects of the upgrade... so it has not all been plain sailing.
PROPOSED SOLUTION: I tend to agree with you that this should be addd as a
breaking change to the current master CHANGES.txt and should be consulted
when people pull a new release. We cannot add this to the release artifacts
however. We would need to roll a new release (1.15.1). If you feel that
this is enough of a reason to roll a new release (which I do not) then
please go ahead and do so.

This is a lesson learned and I can honestly say that it was the result of
us trying to make the upgrade as clean as possible without leaving too much
of the deprecated MR API still around. Maybe this could have however been
phased out across several releases...

Lewis

On Tue, Sep 4, 2018 at 8:53 AM <[hidden email]> wrote:

>
> user Digest 4 Sep 2018 15:53:01 -0000 Issue 2929
>
> Topics (messages 34147 through 34147)
>
> IndexWriter interface in 1.15
>         34147 by: Yossi Tamari
>
> Administrivia:
>
> ---------------------------------------------------------------------
> To post to the list, e-mail: [hidden email]
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> ----------------------------------------------------------------------
>
>
>
>
> ---------- Forwarded message ----------
> From: Yossi Tamari <[hidden email]>
> To: <[hidden email]>
> Cc:
> Bcc:
> Date: Tue, 4 Sep 2018 18:52:54 +0300
> Subject: IndexWriter interface in 1.15
> Hi,
>
>
>
> I missed it at the time, but I just realized (the hard way) that the
> IndexWriter interface was changed in 1.15 in ways that are not backward
> compatible.
>
> That means that any custom IndexWriter implementation will no longer
> compile, and probably will not run either.
>
> I think this was a mistake (maybe a new interface should have been created,
> and the old one deprecated and supported for now, or just the old methods
> deprecated without change, and the new methods provided with a default
> implementation), but it's too late now.
>
> I still think this is something that should be highlighted in the release
> note for 1.15 (meaning at the top, as "breaking changes").
>
> The main changes I encountered:
>
> 1.      setConf and getConf were removed from the interface (without
> deprecation).
> 2.      open was deprecated (that's fine), and its signature was changed
> (from JobConf to Configuration), which means it a completely different
> function technically, and there is no point in the deprecation.
>
>
>
> Yossi.
>
>

--
http://home.apache.org/~lewismc/
http://people.apache.org/keys/committer/lewismc
Reply | Threaded
Open this post in threaded view
|

RE: IndexWriter interface in 1.15

Yossi Tamari
Hi Lewis,

First of all I must say that I can't reproduce my claim regarding getConf/setConf. I was getting a compilation error for their @Override, but not anymore, and it's being called, so I'm not sure what happened.
Open() changing its signature is still a breaking change. I can't roll a new release, because I'm not a maintainer. I'm also not sure if it's justified, because I don't know how many people implement an IndexWriter.
I would still suggest that it be added as a breaking change in master.

        Yossi.

> -----Original Message-----
> From: lewis john mcgibbney <[hidden email]>
> Sent: 06 September 2018 19:00
> To: [hidden email]
> Subject: Re: IndexWriter interface in 1.15
>
> Hi Yossi,
>
> REASON: Upgrade of MapReduce API from legacy to 'new'. This was a breaking
> change for sure and a HUGE patch. We did not however factor in the non-
> braking aspects of the upgrade... so it has not all been plain sailing.
> PROPOSED SOLUTION: I tend to agree with you that this should be addd as a
> breaking change to the current master CHANGES.txt and should be consulted
> when people pull a new release. We cannot add this to the release artifacts
> however. We would need to roll a new release (1.15.1). If you feel that this is
> enough of a reason to roll a new release (which I do not) then please go ahead
> and do so.
>
> This is a lesson learned and I can honestly say that it was the result of us trying
> to make the upgrade as clean as possible without leaving too much of the
> deprecated MR API still around. Maybe this could have however been phased
> out across several releases...
>
> Lewis
>
> On Tue, Sep 4, 2018 at 8:53 AM <[hidden email]> wrote:
>
> >
> > user Digest 4 Sep 2018 15:53:01 -0000 Issue 2929
> >
> > Topics (messages 34147 through 34147)
> >
> > IndexWriter interface in 1.15
> >         34147 by: Yossi Tamari
> >
> > Administrivia:
> >
> > ---------------------------------------------------------------------
> > To post to the list, e-mail: [hidden email] To unsubscribe,
> > e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > ----------------------------------------------------------------------
> >
> >
> >
> >
> > ---------- Forwarded message ----------
> > From: Yossi Tamari <[hidden email]>
> > To: <[hidden email]>
> > Cc:
> > Bcc:
> > Date: Tue, 4 Sep 2018 18:52:54 +0300
> > Subject: IndexWriter interface in 1.15 Hi,
> >
> >
> >
> > I missed it at the time, but I just realized (the hard way) that the
> > IndexWriter interface was changed in 1.15 in ways that are not backward
> > compatible.
> >
> > That means that any custom IndexWriter implementation will no longer
> > compile, and probably will not run either.
> >
> > I think this was a mistake (maybe a new interface should have been created,
> > and the old one deprecated and supported for now, or just the old methods
> > deprecated without change, and the new methods provided with a default
> > implementation), but it's too late now.
> >
> > I still think this is something that should be highlighted in the release
> > note for 1.15 (meaning at the top, as "breaking changes").
> >
> > The main changes I encountered:
> >
> > 1.      setConf and getConf were removed from the interface (without
> > deprecation).
> > 2.      open was deprecated (that's fine), and its signature was changed
> > (from JobConf to Configuration), which means it a completely different
> > function technically, and there is no point in the deprecation.
> >
> >
> >
> > Yossi.
> >
> >
>
> --
> http://home.apache.org/~lewismc/
> http://people.apache.org/keys/committer/lewismc

Reply | Threaded
Open this post in threaded view
|

Re: IndexWriter interface in 1.15

Sebastian Nagel-2
Hi Yossi, hi Lewis,

actually, this is caused by a change of the IndexWriter interface as part of NUTCH-1480 (multiple
index writers of same type). It's reported as a breaking change, but only the need to change the way
how the index writers are configured. Sorry, we've missed to add a note about the need to implement
a new open() method which replaces the old deprecated one.

Nutch is a small project with only a few active committers and we rely on the help of the community
not only for code contributions but also to carefully review and test patches and releases. That's
our bottleneck at present.

Sorry again, there should have been a note about the plugin API change. I think it was a necessary
change because it would be cumbersome to configure multiple Solr index writers using only
configuration properties. The index-writers.xml is easier to maintain.

> because I don't know how many people implement an IndexWriter

It's probably a smaller group. Also: the Java compiler or runtime will complain early about the API
change.

> I would still suggest that it be added as a breaking change in master.

The problem is that the CHANGES.txt is part of the release package. Any change of it will modify the
release signatures and requires a new release. The Apache release policy [1] is very clear in this
respect.

Best,
Sebastian


[1] http://www.apache.org/legal/release-policy.html


On 09/06/2018 06:28 PM, Yossi Tamari wrote:

> Hi Lewis,
>
> First of all I must say that I can't reproduce my claim regarding getConf/setConf. I was getting a compilation error for their @Override, but not anymore, and it's being called, so I'm not sure what happened.
> Open() changing its signature is still a breaking change. I can't roll a new release, because I'm not a maintainer. I'm also not sure if it's justified, because I don't know how many people implement an IndexWriter.
> I would still suggest that it be added as a breaking change in master.
>
> Yossi.
>
>> -----Original Message-----
>> From: lewis john mcgibbney <[hidden email]>
>> Sent: 06 September 2018 19:00
>> To: [hidden email]
>> Subject: Re: IndexWriter interface in 1.15
>>
>> Hi Yossi,
>>
>> REASON: Upgrade of MapReduce API from legacy to 'new'. This was a breaking
>> change for sure and a HUGE patch. We did not however factor in the non-
>> braking aspects of the upgrade... so it has not all been plain sailing.
>> PROPOSED SOLUTION: I tend to agree with you that this should be addd as a
>> breaking change to the current master CHANGES.txt and should be consulted
>> when people pull a new release. We cannot add this to the release artifacts
>> however. We would need to roll a new release (1.15.1). If you feel that this is
>> enough of a reason to roll a new release (which I do not) then please go ahead
>> and do so.
>>
>> This is a lesson learned and I can honestly say that it was the result of us trying
>> to make the upgrade as clean as possible without leaving too much of the
>> deprecated MR API still around. Maybe this could have however been phased
>> out across several releases...
>>
>> Lewis
>>
>> On Tue, Sep 4, 2018 at 8:53 AM <[hidden email]> wrote:
>>
>>>
>>> user Digest 4 Sep 2018 15:53:01 -0000 Issue 2929
>>>
>>> Topics (messages 34147 through 34147)
>>>
>>> IndexWriter interface in 1.15
>>>         34147 by: Yossi Tamari
>>>
>>> Administrivia:
>>>
>>> ---------------------------------------------------------------------
>>> To post to the list, e-mail: [hidden email] To unsubscribe,
>>> e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>> ----------------------------------------------------------------------
>>>
>>>
>>>instead
>>>
>>> ---------- Forwarded message ----------
>>> From: Yossi Tamari <[hidden email]>
>>> To: <[hidden email]>
>>> Cc:
>>> Bcc:
>>> Date: Tue, 4 Sep 2018 18:52:54 +0300
>>> Subject: IndexWriter interface in 1.15 Hi,
>>>
>>>
>>>
>>> I missed it at the time, but I just realized (the hard way) that the
>>> IndexWriter interface was changed in 1.15 in ways that are not backward
>>> compatible.
>>>
>>> That means that any custom IndexWriter implementation will no longer
>>> compile, and probably will not run either.
>>>
>>> I think this was a mistake (maybe a new interface should have been created,
>>> and the old one deprecated and supported for now, or just the old methods
>>> deprecated without change, and the new methods provided with a default
>>> implementation), but it's too late now.
>>>
>>> I still think this is something that should be highlighted in the release
>>> note for 1.15 (meaning at the top, as "breaking changes").
>>>
>>> The main changes I encountered:
>>>
>>> 1.      setConf and getConf were removed from the interface (without
>>> deprecation).
>>> 2.      open was deprecated (that's fine), and its signature was changed
>>> (from JobConf to Configuration), which means it a completely different
>>> function technically, and there is no point in the deprecation.
>>>
>>>
>>>
>>> Yossi.
>>>
>>>
>>
>> --
>> http://home.apache.org/~lewismc/
>> http://people.apache.org/keys/committer/lewismc
>