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

WFLY-16950 Provide a test case for testing EJB/HTTP with a load balancer. #16018

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rachmatowicz
Copy link
Contributor

This pull request provides a test case for using EJB/HTTP with a load balancer.
For more information, see the issue: https://issues.redhat.com/browse/WFLY-16950

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Sep 6, 2022
@bstansberry bstansberry changed the title WFLY-16950 WFLY-16950 Provide a test case for testing EJB/HTTP with a load balancer. Apr 24, 2023
} finally {
HttpClientUtils.closeQuietly(response);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rachmatowicz this test just logs which node receives the HTTP request but doesn't really check any specific behavior: e.g. checking two subsequent HTTP requests are handled by the same node

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. All of these tests were set up to merely observe where the requests were going, and don't really represent validations of correct behavior. I was using it when testing my implementations of the session affinity management. I once I see that the affinity management implementation is working correctly, I will add in the constraints for validating behavior. In particular, if you run the test as is, you will see that the EJB/HTTP implementation does not do what it is supposed to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left the test in Draft state for that reason.

int count = 1;
for (int i = 0; i < COUNT; ++i) {
result = slsb.increment();
log.info("Called SLSBWithoutFailover: SLSB backend node = " + result.getNode());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rachmatowicz like the testLoadBalancer test, this one just logs what node handles the EJB request, doesn't enforce any behavior; I guess, since we are invoking SLSB, that we expect requests to be load balanced across nodes (no sticky session)

for (int i = 0; i < COUNT; ++i) {
result = sfsb.increment();
log.infof("Called SFSBWithoutFailover: SFSB value = %s, backend node = %s", result.getValue().intValue(), result.getNode());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rachmatowicz I guess, since we have SFSB, that sticky session applies here; is that the expected behavior?

log.infof("Called SFSBAndSLSBWithoutFailover: SFSB value = %s, backend node = %s", sfsbResult.getValue().intValue(), sfsbResult.getNode());

slsbResult = slsb.increment();
log.infof("Called SFSBAndSLSBWithoutFailover: SLSB backend node = %s", slsbResult.getNode());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rachmatowicz perhaps this test might also cover what's tested in testSLSBWithoutFailover and testSFSBWithoutFailover and we can remove them.... or, maybe, I am missing some detail which differentiates the three of them?

@rhusar
Copy link
Member

rhusar commented Oct 30, 2023

@rachmatowicz Needs a rebase...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
6 participants