Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISPN-7636 Update default HR version and fix HR decoder #4989

Merged
merged 1 commit into from Mar 21, 2017

Conversation

vjuranek
Copy link
Member

https://issues.jboss.org/browse/ISPN-7636

  • Use HR 2.6 as default protocol version
  • Fix HR decoder to read add listener command correctly

@gustavocoding
Copy link

There some test failures in the Hot Rod client suite:
org.junit.ComparisonFailure: expected:<...protocol version: 2.[5]> but was:<...protocol version: 2.[6]>

@wburns
Copy link
Member

wburns commented Mar 17, 2017

It sounds like we need a test that verifies the add client listener behavior in the decoder. As long as this test is added to a test that is setup using HotRodTestingUtils to configure the server it will find if there is any issue.

* Use HR 2.6 as default protocol version
* Fix HR decoder to read add listener command correctly
@vjuranek
Copy link
Member Author

I'm on PTO, will look on dedicated next week (failing test is fixed).

@wburns
Copy link
Member

wburns commented Mar 21, 2017

Actually looking closer as you mentioned on ISPN-7636 the ClusterInvalidatedNearCacheTest uses the proper test utils. But for some reason the test actually passes fine for me without hanging. I just would love for a test to hang to make sure we plugged all the holes properly when the fix is applied.

@wburns
Copy link
Member

wburns commented Mar 21, 2017

Okay I see the issue [1] added additional lines to the code but didn't add the appropriate read from the context. It used to not store to the context for the converter information because it was the last one and just returned. With the new protocol version it became the last bytes on the stream for add listener if it is greater than version 2.5.

So I am able to get it to hang with version 2.6 change as well.

[1] vjuranek@5af2fd5#diff-a4a6292d50595775143111292bcf73afR413

@wburns
Copy link
Member

wburns commented Mar 21, 2017

Pulling...

@wburns wburns merged commit 6d92d9a into infinispan:master Mar 21, 2017
@wburns
Copy link
Member

wburns commented Mar 21, 2017

Integrated into master, thanks @vjuranek !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants