-
Notifications
You must be signed in to change notification settings - Fork 69
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
Enable protostream #853
Enable protostream #853
Conversation
* Enables protostream encoding in the external Infinispan * Changes the testsuite to use the Hot Rod client Closes keycloak#628 Signed-off-by: Pedro Ruivo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank @pruivo, this PR looks good to me.
To the errors you see. What were you testing this against? If you tested against runner-keycloak
namespace created by the nightly run, it is probably caused by the fact the testsuite currently does not work with A/A. We need to test this against A/P deployment where the testsuite is passing.
Here are the errors: https://github.com/keycloak/keycloak-benchmark/actions/runs/9559243516/job/26354830531
I will be looking at the A/A failures now.
@@ -11,6 +11,7 @@ | |||
|
|||
<properties> | |||
<keycloak.version>999.0.0-SNAPSHOT</keycloak.version> | |||
<infinispan.version>15.0.4.Final</infinispan.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deployment is currently using 15.0.x Infinispan image, I believe this can break some API compatibility. Is there some nightly maven release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That version is only used by the REST client since the Hot Rod client will use the version shipped with Keycloak. It is fine and only the major version matters.
I created a new cluster in a different namespace (keycloak + infinispan) with Route53. I was using the existing aurora DB instead of creating and populating another one. I assume it wouldn't have any impact. |
I suspect it can be caused by the database. It seems A/A tests leave the database in a state where some requests end up with 500. Can you please test with a different database cluster? |
@mhajas thanks for the tip. I'll change this to ready for review 👍
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @pruivo!
Closes #628
I have the following error with status code 500. I haven't figured out where they come from. I couldn't find any exceptions in the pod's log.
We probably need to fork the
main
branch before merging this one. The forked branch will keep compatibility with jboss-marshalling and KC25.