ptupitsyn commented on code in PR #11003:
URL: https://github.com/apache/ignite/pull/11003#discussion_r1374654659
##########
docs/_docs/services/services.adoc:
##########
@@ -234,6 +234,22 @@ tab:C++[]
//== Accessing Services from Compute Tasks
// TODO the @ServiceResource annotation
+== Service Awareness [[service_awareness]]
+For link:../thin-clients/java-thin-client.adoc#java_thin_client[Java Thin
Client] you can activate Service Awareness.
+To do that, enable
link:../thin-clients/java-thin-client.adoc#partition_awareness[Partition
Awareness].
+
+When used, the thin client knows about nodes with the service instances and
invokes them directly.
+
+Without Service Awareness, the invocation requests are sent to a random node.
If it has no service
+instance deployed, the request is redirected to a proper node. Such
redirection costs.
+
+[NOTE]
+====
+The service topology is updated asynchronously starting with the first service
invocation.
+Thus, few invocation redirects on the server nodes are still possible.
Especially if the service migrates or was redeployed.
Review Comment:
```suggestion
Thus, some invocation redirects are still possible.
```
##########
docs/_docs/services/services.adoc:
##########
@@ -234,6 +234,22 @@ tab:C++[]
//== Accessing Services from Compute Tasks
// TODO the @ServiceResource annotation
+== Service Awareness [[service_awareness]]
+For link:../thin-clients/java-thin-client.adoc#java_thin_client[Java Thin
Client] you can activate Service Awareness.
+To do that, enable
link:../thin-clients/java-thin-client.adoc#partition_awareness[Partition
Awareness].
+
+When used, the thin client knows about nodes with the service instances and
invokes them directly.
+
+Without Service Awareness, the invocation requests are sent to a random node.
If it has no service
+instance deployed, the request is redirected to a proper node. Such
redirection costs.
Review Comment:
```suggestion
Without Service Awareness, the invocation requests are sent to a random
node. If it has no service
instance deployed, the request is redirected to a different node. This
additional network hop adds overhead.
With Service Awareness, the thin client knows where service instances are
deployed and sends the request to the correct node.
```
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientServicesImpl.java:
##########
@@ -281,6 +344,96 @@ else if
(ch.clientChannel().protocolCtx().isFeatureSupported(ProtocolBitmaskFeat
}
}
+ /**
+ * Asynchronously requests the service topology if the partition awareness
is enabled and if the update is requred.
+ *
+ * @param srvcTop If not {@code null}, uses this service topology record
and ignores {@code name}.
+ * @param name If {@code srvTop} is {@code null}, gets service topology
record by this name.
+ * @see #needUpdateSrvcTop(ServiceTopology, AffinityTopologyVersion)
+ */
+ private void tryRequestServiceTopology(@Nullable ServiceTopology srvcTop,
String name) {
+ if (srvcTop == null) {
+ srvcTop = servicesTopologies.compute(name, (nm, t) -> {
+ if (t == null)
+ t = new ServiceTopology();
+
+ return t;
+ });
+ }
+
+ ServiceTopology srvcTop0 = srvcTop;
+
+ AffinityTopologyVersion curAffTop =
ch.affinityContext().lastTopology().version();
+
+ if (!needUpdateSrvcTop(srvcTop0, curAffTop))
+ return;
+
+ ForkJoinPool.commonPool().execute(() -> {
+ Throwable t = null;
+
+ try {
+ if (log.isDebugEnabled())
+ log.debug("Requesting service topology update for the
service '" + name + "' ...");
+
+ List<UUID> nodes = ch.service(
Review Comment:
Let's use `serviceAsync`, so we don't need a separate thread.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientServicesImpl.java:
##########
@@ -281,6 +344,96 @@ else if
(ch.clientChannel().protocolCtx().isFeatureSupported(ProtocolBitmaskFeat
}
}
+ /**
+ * Asynchronously requests the service topology if the partition awareness
is enabled and if the update is requred.
+ *
+ * @param srvcTop If not {@code null}, uses this service topology record
and ignores {@code name}.
+ * @param name If {@code srvTop} is {@code null}, gets service topology
record by this name.
+ * @see #needUpdateSrvcTop(ServiceTopology, AffinityTopologyVersion)
+ */
+ private void tryRequestServiceTopology(@Nullable ServiceTopology srvcTop,
String name) {
+ if (srvcTop == null) {
+ srvcTop = servicesTopologies.compute(name, (nm, t) -> {
+ if (t == null)
+ t = new ServiceTopology();
+
+ return t;
+ });
+ }
+
+ ServiceTopology srvcTop0 = srvcTop;
+
+ AffinityTopologyVersion curAffTop =
ch.affinityContext().lastTopology().version();
+
+ if (!needUpdateSrvcTop(srvcTop0, curAffTop))
+ return;
+
+ ForkJoinPool.commonPool().execute(() -> {
+ Throwable t = null;
+
+ try {
+ if (log.isDebugEnabled())
+ log.debug("Requesting service topology update for the
service '" + name + "' ...");
+
+ List<UUID> nodes = ch.service(
+ ClientOperation.SERVICE_GET_TOPOLOGY,
+ req -> {
+ if
(!req.clientChannel().protocolCtx().isFeatureSupported(ProtocolBitmaskFeature.SERVICE_TOPOLOGY))
+ throw new
ClientFeatureNotSupportedByServerException(ProtocolBitmaskFeature.SERVICE_TOPOLOGY);
Review Comment:
If the feature is not supported by the server, we should not throw an
exception. Just skip the service awareness logic.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientOperation.java:
##########
@@ -280,7 +280,10 @@ public enum ClientOperation {
OP_SET_ITERATOR_START(9022),
/** IgniteSet.iterator page. */
- OP_SET_ITERATOR_GET_PAGE(9023);
+ OP_SET_ITERATOR_GET_PAGE(9023),
+
+ /** Get service topolog. */
+ SERVICE_GET_TOPOLOGY(9024);
Review Comment:
Please move up to other service-related operations, and use `7003` code
(make sure to update the IEP as well)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
