HSEARCH-668 Transactional JMS

[emmanuelbernard]

The tests don't work. It seems the JMS messages are not sent which leads to the test failures.
It could be for many reasons:

• test is not doing what I think it does
• configuration of the test is incorrect (anything to do to make a transactional queue on the client side?, ...)
• the actual core code does not do what I think it does
  • is the Hibernate flush actually called before the beforeTransaction of Hibernate Search
  • is the JMS calls to a transactional queue correct?

I need a second pair of eyes, I'm out of ideas on how and what to look.

[DavideD]

I think you should use VersionTestHelper instead of Version

[emmanuelbernard]

VersionTestHelper.injectVariables( "${dependency.version.HibernateSearch}" );?
I don't see a non public method to access the Hibernate Search version

[emmanuelbernard]

looks like it did the trick and the failure in the IDE is now consistent with the CLI. I've made one of the private method public

[emmanuelbernard]

I've rebased to fix the massive copy paste error. Now the afterCompletion code is empty

[emmanuelbernard]

Pushed by rebasing to latest master, fixing IDE tests and removing isTraceEnabled.
Still the tests fail and I don't know why. Help welcome.

[emmanuelbernard]

I finally found the issue. It was a configuration problem as expected. One need to use the right JNDI bound XA JMS connection factory or else it does not do what you want silently. Of course, documentation on the internet is very sparse.

Unfortunately my test (test the non addition of an entry) takes ~35s extra on the test suite. If someone can find a better approach, that would be better.

The documentation is missing but I'd rather get a go (and even a push) before looking into this.
Documentation wise I think we need to say that:

• it's only useful when using JMS as a backend and when on a JTA environment. Otherwise our current post commit approach is semantically as good or close to
  • only needed on JMS client side, the master does not need / should not have a tx worker.
• the benefit is an all or nothing to the DB changes and the index change request
• it also makes it a bit simpler to follow the system generally

If you look at the code, you'll see that a single property is necessary. Kind of nice.

Marked as urgent otherwise Hardy's PR will mess up mine ;)

[Sanne]

One need to use the right JNDI bound XA JMS connection factory or else it does not do what you want silently.

In this case it's just our tests, and we could debug it. End users will have to match this up correctly using their own queue names.. is there really no way to WARN if we're taking the wrong kind from JNDI?

only needed on JMS client side, the master does not need / should not have a tx worker

Shouldn't we make an option to make consumptions transactional as well? i.e. if the master takes a batch of Works A, then can not manage to flush that successfully to the IndexWriter, and fails, the next master node should still get A in his TODO list?
Certainly a different issue.

The documentation is missing but I'd rather get a go (and even a push) before looking into this.

+1 to break up in other tasks

If you look at the code, you'll see that a single property is necessary. Kind of nice.

Awesome!

[DavideD]

I see that you are only testing this on "slave-2", would it make sense to check from "slave-1" and "master" as well?

[emmanuelbernard]

No I don't see any reason. It would only slow down the test.

[DavideD]

superseeded by #862