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.

Sign up for GitHub

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

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

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
6c2c255
[WEJBHTTP-106] Add support for multiple version of HTTP Handlers for …
fl4via Jan 10, 2023
75162be
[WEJBHTTP-106] Move HttpServiceConfig from enum to a simple class.
fl4via Jan 10, 2023
782f815
[WEJBHTTP-81] Introduce handler versioning scaffolding.
rachmatowicz Mar 8, 2023
fddb756
[WEJBHTTP-81] Add HttpStickinessHandler interface to HttpTargetContext.
rachmatowicz Aug 2, 2022
75b0292
[WEJBHTTP-81] Convert inline ClientCallbacks in HttpTargetContext to …
rachmatowicz Sep 27, 2022
a359156
[WEJBHTTP-81] Add in HTTPInvoker handler to discover backend server.
rachmatowicz Sep 28, 2022
c0b9a42
[WEJBHTTP-81] Add some comments for digest authentication processing.
rachmatowicz Oct 4, 2022
5c637e4
[WEJBHTTP-81] Remove legacy affinity concept.
rachmatowicz Oct 4, 2022
9e059aa
[WEJBHTTP-81] Comments to clarify handler processing stages.
rachmatowicz Oct 5, 2022
c4d7336
[WEJBHTTP-81] Allow stickiness handlers to throw exceptions.
rachmatowicz Oct 7, 2022
6252596
[WEJBHTTP-81] Introduce transaction stickiness map.
rachmatowicz Oct 4, 2022
88560f4
[WEJBHTTP-81] Add in routing support for processing routes.
rachmatowicz Oct 7, 2022
2fdb50f
[WEJBHTTP-81] Add in EJB stickiness handling and bump protocol/handle…
rachmatowicz Oct 7, 2022
d2383e7
[WEJBHTTP-81] Fix protocol version to handler version mismatch.
rachmatowicz Mar 10, 2023
891ea3a
[WEJBHTTP-81] Fix SFSB test cases.
rachmatowicz Mar 11, 2023
a4b8e61
Fixes from review.
rachmatowicz Mar 15, 2023
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
8 changes: 7 additions & 1 deletion common/src/main/java/org/wildfly/httpclient/common/Version.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,18 @@
/*
* Versioning enum for HttpHandler implementations.
*
* TODO: due to the way EENamespaceInteroperability.createInteroperabilityHandler(HttpHandler...) works,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I see your point. It is the drawback of introducting the handler version after protocol version is already at number 2. I'm still open to suggestions if you would do it differently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm thinking about it...

* the original protocol versions JAVAEE_PROTOCOL_VERSION and JAKARTA_PROTOCOL_VERSION need to share
* the same handler instance, so it was not possible to match protocol handler indexes to protocol versions
* in a 1-to-1 manner. In order to avoid a confusing protocol version to handler version mismatch, they share
* the handler installed at index 2.
* TODO: integrate this with the Protocol class in a nice way.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How can this be integrated if the idea was to decouple the protocol version from the handlers version?
Maybe we should rename this Version class to HandlerVersion to make it clearer?
And how does this interplay with incremented handler versions in the future, supposing only one type of handler needs to have its version incremented? Say, for example, that only Transaction handlers will need a new request header, or a new parameter, or anything really that is valid only for transaction handlers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have renamed the class. These are good questions. And what if there are Protocol version changes which don't correspond to handler version changes.

Copy link
Collaborator Author

@rachmatowicz rachmatowicz Mar 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I just lost a well-crafted 200 word response to this (with one misplaced click which killed my text :-()
I am looking at this again and here is my idea in a nutshell:

  1. introduce one Handler class for each of org.wildfly.httpclient.{ejb, transaction, naming} which will contain the handler version for that module, just as the Protocol class holds the protocol version to be used in org.wildfly.httpclient.common. Handler.LATEST serves the same purpose. Use this version when building client requests for ejb, transaction and naming invocations. In other words, the version in the context path is the handler version.
  2. In the handshake between ClientConnectionHolder, the server-side wrapper functionality of EENamespaceInteroperabilityHandler / JakartaNamespaceHandler and EEInteroperableClientExchange, set things up so that only one combined, context path-independent wrapper is used on the server side, and it is based only on reading the x-wf-version HTTP header to decide which marshaller and unmarshaller factories to attach to requests and replies. In other words, the protocol version is only visible as a HTTP header. When protocol changes occur, the new version and associated changes are confined to those classes: Protocol, ClientConnectionHolder, the new wrapper class, and EEInteroperableClientExchange.

I'm going to have a shot at creating an implementation of this; well, on second thought, it might be better to have a short chat about it first.

*
* @author Richard Achmatowicz
*/
public enum Version {
VERSION_1(1),
EARLIEST(2),
VERSION_1(2),
VERSION_2(2),
LATEST(3)
;
Expand Down
2 changes: 1 addition & 1 deletion ejb/src/main/java/org/wildfly/httpclient/ejb/EjbHttpService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public HttpHandler createHttpHandler() {
RequestEncodingHandler requestEncodingHandler = new RequestEncodingHandler(encodingHandler);
requestEncodingHandler.addEncoding(Headers.GZIP.toString(), GzipStreamSourceConduit.WRAPPER);

int versionIndex = version.getVersion()-1;
int versionIndex = version.getVersion() - Version.EARLIEST.getVersion();
requestEncodingHandlers[versionIndex] = requestEncodingHandler;
}
return httpServiceConfig.wrap(requestEncodingHandlers);
Expand Down
2 changes: 1 addition & 1 deletion naming/src/main/java/org/wildfly/httpclient/naming/HttpRemoteNamingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public HttpHandler createHandler() {
routingHandler.add(Methods.PATCH, RENAME_PATH + nameParamPathSuffix, new RenameHandler(version));
routingHandler.add(Methods.PUT, CREATE_SUBCONTEXT_PATH + nameParamPathSuffix, new CreateSubContextHandler(version));

int versionIndex = version.getVersion()-1;
int versionIndex = version.getVersion() - Version.EARLIEST.getVersion();
handlers[versionIndex] = new BlockingHandler(new ElytronIdentityHandler(routingHandler));
}
return httpServiceConfig.wrap(handlers);
Expand Down
2 changes: 1 addition & 1 deletion ...action/src/main/java/org/wildfly/httpclient/transaction/HttpRemoteTransactionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public HttpHandler createHandler() {
routingHandler.add(Methods.POST, XA_ROLLBACK_PATH, new XARollbackHandler(version));
routingHandler.add(Methods.GET, XA_RECOVER_PATH, new XARecoveryHandler(version));

int versionIndex = version.getVersion()-1;
int versionIndex = version.getVersion() - Version.EARLIEST.getVersion();
handlers[versionIndex] = new BlockingHandler(new ElytronIdentityHandler(routingHandler));
}
return httpServiceConfig.wrap(handlers);
Expand Down