[
https://issues.apache.org/jira/browse/TINKERPOP-3213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18035185#comment-18035185
]
ASF GitHub Bot commented on TINKERPOP-3213:
-------------------------------------------
kenhuuu commented on code in PR #3258:
URL: https://github.com/apache/tinkerpop/pull/3258#discussion_r2488162759
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java:
##########
@@ -773,6 +773,80 @@ public Client alias(final Map<String, String> aliases) {
}
}
+ /**
+ * A {@code Client} implementation that operates in the context of a
session. {@code ChildSessionClient} is tied to
+ * another client as a child, it borrows the connection from the parent
client's pool for the transaction. Requests are
+ * sent to a single server, where each request is bound to the same thread
and same connection with the same set of
+ * bindings across requests.
+ * Transaction are not automatically committed. It is up the client to
issue commit/rollback commands.
+ */
+ public final static class SessionedChildClient extends Client {
+ private final String sessionId;
+ private final boolean manageTransactions;
+ private final boolean maintainStateAfterException;
+ private final Client parentClient;
+ private Connection borrowedConnection;
+ private boolean closed;
+
+ public SessionedChildClient(final Client parentClient, String
sessionId) {
+ super(parentClient.cluster, parentClient.settings);
+ this.parentClient = parentClient;
Review Comment:
Does it make sense for any type of Client to be the parent? SessionedClient
only allows for one connection so maybe there should be a check here that
disallows that particular client?
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java:
##########
@@ -773,6 +773,80 @@ public Client alias(final Map<String, String> aliases) {
}
}
+ /**
+ * A {@code Client} implementation that operates in the context of a
session. {@code ChildSessionClient} is tied to
+ * another client as a child, it borrows the connection from the parent
client's pool for the transaction. Requests are
+ * sent to a single server, where each request is bound to the same thread
and same connection with the same set of
+ * bindings across requests.
+ * Transaction are not automatically committed. It is up the client to
issue commit/rollback commands.
+ */
+ public final static class SessionedChildClient extends Client {
Review Comment:
It would have been best if there was a solution that kept this class
package-private, as it now differs from the other Clients that are only
supposed to be exposed via Cluster. It's fine for this PR, but I'd probably
recommend adding a Jira to look into this again in the future after this is
merged.
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java:
##########
@@ -773,6 +773,80 @@ public Client alias(final Map<String, String> aliases) {
}
}
+ /**
+ * A {@code Client} implementation that operates in the context of a
session. {@code ChildSessionClient} is tied to
+ * another client as a child, it borrows the connection from the parent
client's pool for the transaction. Requests are
+ * sent to a single server, where each request is bound to the same thread
and same connection with the same set of
+ * bindings across requests.
+ * Transaction are not automatically committed. It is up the client to
issue commit/rollback commands.
+ */
+ public final static class SessionedChildClient extends Client {
+ private final String sessionId;
+ private final boolean manageTransactions;
+ private final boolean maintainStateAfterException;
+ private final Client parentClient;
+ private Connection borrowedConnection;
+ private boolean closed;
+
+ public SessionedChildClient(final Client parentClient, String
sessionId) {
+ super(parentClient.cluster, parentClient.settings);
+ this.parentClient = parentClient;
+ this.sessionId = sessionId;
+ this.closed = false;
+ this.manageTransactions = parentClient.settings.getSession().map(s
-> s.manageTransactions).orElse(false);
+ this.maintainStateAfterException =
parentClient.settings.getSession().map(s ->
s.maintainStateAfterException).orElse(false);
+ }
+
+ public String getSessionId() {
Review Comment:
This method is currently unused which may be a potential problem. There is
specialized handling for sessions at
https://github.com/apache/tinkerpop/blob/3.7-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java#L313
as well as
https://github.com/apache/tinkerpop/blob/3.7-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java#L235
> Re-use connections for sessions
> -------------------------------
>
> Key: TINKERPOP-3213
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3213
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
> Affects Versions: 3.7.4
> Reporter: Ken Hu
> Priority: Major
> Fix For: 3.8.0, 3.7.5
>
>
> The Java driver creates a new client for each session. These sessions are
> used for transactions as well. In situations where there are lots of
> transactions being used, the driver will open and close connections per
> transaction. It would be best if this overhead was reduced by reusing exising
> connections.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
