[hibernate-dev] HSEARCH-1270 : indexing (still) happening after Session#clear()

[Sanne Grinovero]

On 1 October 2013 19:41, Hardy Ferentschik <hardy at hibernate.org> wrote:
>
> On 1 Jan 2013, at 8:09 PM, Sanne Grinovero <sanne at hibernate.org> wrote:
>
>> On 1 October 2013 15:41, Hardy Ferentschik <hardy at hibernate.org> wrote:
>>>
>>> On 30 Jan 2013, at 11:00 PM, Sanne Grinovero <sanne at hibernate.org> wrote:
>>>
>>>> I'm having a half-baked solution for HSEARCH-1270, and it looks like I
>>>> can't do better than that without waiting for HHH-8577 to be included
>>>> in ORM [JIRA links at the bottom].
>>>
>>> Can you make a use case example. Clearing the queue makes only sense in the
>>> case where the indexing API is used, right?
>>
>> No, it's a general problem. Pseudo-code inspired from my testcase:
>>
>>   FullTextSession s = Search.getFullTextSession( openSession() );
>>   s.getTransaction().begin();
>>   persistDrink( s, "Water" );
>>   s.clear();
>>   persistDrink( s, "Espresso" );
>>   s.getTransaction().commit();
>>
>>   void persistDrink(Session s, String drinkName) {
>>      Drink d = new Drink();
>>      d.setName( drinkName );
>>      s.persist( d );
>>   }
>>
>> If you run that, two Drinks "water" and "espresso" will be indexed,
>> but only espresso is committed to the database. Nasty.
>
> Really? So assuming event based indexing is occurring there. We won't get the event until
> commit is called, right?

No, we get the persist event right away, and we store it in a queue
which is associated to the transaction.
We then process the queue when the transaction is committed.
There is no event for the s.clear() invocation, so we'll never be
aware that some operations should have been dropped.

> So you are saying in this case we are still getting two PostInsertEvents
> even though the first event got "cleared". Btw, is there a difference between persist and save?
> I randomly took LuceneQueryTest and modified the test as you described. At least with save
> I only got a single event (for the book created after the clear).

Could you share your test, this is not matching my observations.
Note that my tests use projections to assert the index state, as a
missing entry in the database is not going to be loaded by a
FullTextQuery.


>>>> # The problem described by HSEARCH-1270
>>>> After the user has enqueued some indexing operations in Hibernate
>>>> Search's Worker, if he then invokes Session.clear() the queue is not
>>>> cleared.
>>>
>>> Not sure I would expect that Session.clear has in impact on the manually queued
>>> indexing objects.
>>>
>>>> #What now
>>>> Considering that we can't do better than that - at least without
>>>> patching ORM with HHH-8577 - would you still merge such a patch?
>>>
>>> No. I am not yet convinced that Session.clear should have the semantics you are
>>> implying.
>>
>> Does the above example affect your opinion?
>
> No. See above.
>
>>>> Another approach to the problem could be to provide an explicit
>>>>
>>>>  FullTextSession#clearIndexingQueue()
>>>
>>> I think that is the better solution.
>>
>> I'll send a pull request to provide this as a new feature, but it
>> doesn't technically resolve
>> the HSEARCH-1270 issue: we're not accurately matching all operations happening
>> on the database, which I consider a violation of the project promise.
>
> This depends on whether automatic indexing is working as it should. Using Session.save
> I was not able to re-produce the semantics you describe. I am not sure yet whether persist
> behaves differently.
>
> Assuming that you indeed get two PostInsertEvents in your scenario, I would argue that
> we are dealing with a bug in ORM which should be addressed there.
>
>> Good to have #clearIndexingQueue() anyway as there might be cases in which it's
>> useful even not just as a workaround: there have been feature requests
>> to temporarily disable indexing on a Session for performance reasons.
>
> ok
>
>> Moving HSEARCH-1270 out of the 4.4 plans, now replaced by:
>>  HSEARCH-1425 - Introduce a FullTextSession.clearIndexingQueue method
>
> Why did you move it out?

Because I can't fix it to automatically respect the clear() operation,
which is what the issue is about.
We're proposing an alternative, which got its own issue number.

> IMO we would provide  FullTextSession.clearIndexingQueue for
> HSEARCH-1270 and that's it.

-1 they are different, and while I can't fix HSEARCH-1270 now I can
send a PR for HSEARCH-1425,
so I'm tracking them separately.
We're discussing above if we should reject HSEARCH-1270: I am not
convinced that we should as
it causes the index to get out of synch with the database, but the
good news is that we don't need to find
an agreement before 4.4.0.Final and before HSEARCH-1425, so you see
plenty of good reasons
to track the two things separately ;-)

-- Sanne

>
> --Hardy