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

Fix session affinity management (stickiness) in wildfly-http-client #108

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,6 @@ interface HttpClientMessages extends BasicLogger {
@Message(id = 14, value = "JavaEE to JakartaEE backward compatibility layer have been installed")
void javaeeToJakartaeeBackwardCompatibilityLayerInstalled();

@Message(id = 15, value = "Failed to acquire backend server")
RuntimeException failedToAcquireBackendServer(@Cause Throwable e);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import io.undertow.util.Cookies;
import io.undertow.util.HeaderValues;
import io.undertow.util.Headers;
import io.undertow.util.HttpString;
import io.undertow.util.Methods;
import io.undertow.util.StatusCodes;
import org.jboss.marshalling.InputStreamByteInput;
Expand All @@ -46,12 +47,14 @@
import java.io.ObjectInput;
import java.io.OutputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.security.AccessController;
import java.security.GeneralSecurityException;
import java.security.PrivilegedAction;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.zip.GZIPInputStream;
Expand All @@ -71,7 +74,7 @@ public class HttpTargetContext extends AbstractAttachable {
}

private static final String EXCEPTION_TYPE = "application/x-wf-jbmar-exception";

private static final HttpString BACKEND_HEADER = new HttpString("Backend");
private static final String JSESSIONID = "JSESSIONID";

private final HttpConnectionPool connectionPool;
Expand Down Expand Up @@ -144,6 +147,49 @@ private void acquireSessionAffinity(CountDownLatch latch, AuthenticationConfigur
}, null, latch::countDown);
}

public URI acquireBackendServer() throws Exception {
// HttpClientMessages.MESSAGES.info("HtpTargetContext: acquireBackendServer()");
rachmatowicz marked this conversation as resolved.
Show resolved Hide resolved
return acquireBackendServer(AUTH_CONTEXT_CLIENT.getAuthenticationConfiguration(uri, AuthenticationContext.captureCurrent()));
}

private URI acquireBackendServer(AuthenticationConfiguration authenticationConfiguration) throws Exception {
ClientRequest clientRequest = new ClientRequest();
clientRequest.setMethod(Methods.GET);
clientRequest.setPath(uri.getPath() + "/common/v1/backend");
AuthenticationContext context = AuthenticationContext.captureCurrent();
SSLContext sslContext;
try {
sslContext = AUTH_CONTEXT_CLIENT.getSSLContext(uri, context);
} catch(GeneralSecurityException e) {
HttpClientMessages.MESSAGES.failedToAcquireBackendServer(e);
return null;
}

// returns a URI of the form <scheme>://<host>:<port>?name=<jboss.node.name>
// this permits having access to *both* the IP:port and the hostname identifiers for the server
CompletableFuture<URI> result = new CompletableFuture<>();
sendRequest(clientRequest, sslContext, authenticationConfiguration,
null,
null,
((resultStream, response, closeable) -> {
HeaderValues backends = response.getResponseHeaders().get(BACKEND_HEADER);
if (backends == null) {
result.completeExceptionally(HttpClientMessages.MESSAGES.failedToAcquireBackendServer(new Exception("Missing backend header on response")));
}
try {
String backendString = backends.getFirst();
URI backendURI = new URI(backendString);
result.complete(backendURI);
} catch(URISyntaxException use) {
result.completeExceptionally(HttpClientMessages.MESSAGES.failedToAcquireBackendServer(use));
} finally {
IoUtils.safeClose(closeable);
}
}),
result::completeExceptionally, null, null);
return result.get();
}

public void sendRequest(ClientRequest request, SSLContext sslContext, AuthenticationConfiguration authenticationConfiguration, HttpMarshaller httpMarshaller, HttpStickinessHandler httpStickinessHandler, HttpResultHandler httpResultHandler, HttpFailureHandler failureHandler, ContentType expectedResponse, Runnable completedTask) {
sendRequest(request, sslContext, authenticationConfiguration, httpMarshaller, httpStickinessHandler, httpResultHandler, failureHandler, expectedResponse, completedTask, false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ protected SessionID createSession(final EJBReceiverSessionCreationContext receiv
targetContext.awaitSessionId(true, authenticationConfiguration);
CompletableFuture<SessionID> result = new CompletableFuture<>();

// debugging
URI backendURI = targetContext.acquireBackendServer();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the best way to add this here? I think that having a blocking invocation at the HttpEJBReceiver will just cause the receiver thread to hold while the other end is responding and this could lead to numerous blocking thread issues.
Ideally, we need a way of having this information available before createSession kicks in, or at least an analysis (maybe you already did that analysis and already know the answer to this question) of why this won't cause thread issues.

Copy link
Collaborator Author

@rachmatowicz rachmatowicz Mar 15, 2023

Choose a reason for hiding this comment

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

If that's the case, then someone else needs to have a look at this particular issue and do an analysis. It would need to be you or Richard or Paul. The problem is that the code for initializing a transaction and marshaling the transactional state into the invocation (in HttpTargetContext.writeTransaction()) is extremely tightly coupled, and didn't easily allow initializing the transaction (on a backend server chosen by the load balancer) and then using the resulting URI as a target for the invocation.

EjbHttpClientMessages.MESSAGES.infof("HttpEJBReceiver: Getting backend server URI: %s", backendURI);

HttpEJBInvocationBuilder builder = new HttpEJBInvocationBuilder()
.setInvocationType(HttpEJBInvocationBuilder.InvocationType.STATEFUL_CREATE)
.setAppName(locator.getAppName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.wildfly.httpclient.ejb;

import io.undertow.util.Headers;
import io.undertow.util.HttpString;
import org.jboss.ejb.client.EJBClient;
import org.jboss.ejb.client.EJBClientContext;
import org.jboss.ejb.client.EJBClientInvocationContext;
Expand Down Expand Up @@ -57,6 +58,7 @@ public class SimpleInvocationTestCase {
@Before
public void before() {
EJBTestServer.registerServicesHandler("common/v1/affinity", httpServerExchange -> httpServerExchange.getResponseHeaders().put(Headers.SET_COOKIE, "JSESSIONID=" + EJBTestServer.INITIAL_SESSION_AFFINITY));
EJBTestServer.registerServicesHandler("common/v1/backend", httpServerExchange -> httpServerExchange.getResponseHeaders().put(new HttpString("Backend"), EJBTestServer.getDefaultServerURL()+"?node=localhost" ));
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 10000; ++i) {
sb.append("Hello World ");
Expand Down