ptupitsyn commented on code in PR #6391:
URL: https://github.com/apache/ignite-3/pull/6391#discussion_r2266648663
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/compute/ClientComputeExecuteColocatedRequest.java:
##########
@@ -73,10 +73,10 @@ public static CompletableFuture<ResponseWriter> process(
return readTableAsync(tableId, tables).thenCompose(table ->
readTuple(schemaId, noValueSet, tupleBytes, table, true)
.thenCompose(keyTuple -> {
- Builder metadataBuilder =
ComputeEventMetadata.builder(Type.SINGLE)
+ ComputeEventMetadataBuilder metadataBuilder =
ComputeEventMetadata.builder(Type.SINGLE)
+ .eventUser(clientContext.userDetails())
.tableName(table.name())
-
.initiatorClient(clientContext.remoteAddress().toString())
- .eventUser(clientContext.userDetails());
+
.clientAddress(clientContext.remoteAddress().toString());
Review Comment:
Can we pass `remoteAddress` as is, without string conversion? It might be
more useful this way and we'll have one less allocation.
##########
modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ProtocolBitmaskFeature.java:
##########
@@ -77,7 +77,12 @@ public enum ProtocolBitmaskFeature {
/**
* Direct mapping for SQL queries within explicit transactions.
*/
- SQL_DIRECT_TX_MAPPING(10);
+ SQL_DIRECT_TX_MAPPING(10),
+
+ /**
+ * Compute events.
+ */
+ COMPUTE_EVENTS(11);
Review Comment:
I suggest `COMPUTE_TASK_ID`, because that is what we add to the protocol.
There is nothing about the events.
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/compute/ClientComputeExecutePartitionedRequest.java:
##########
@@ -65,12 +67,14 @@ public static CompletableFuture<ResponseWriter> process(
int partitionId = in.unpackInt();
Job job = ClientComputeJobUnpacker.unpackJob(in,
clientContext.hasFeature(PLATFORM_COMPUTE_JOB));
+ UUID taskId = clientContext.hasFeature(COMPUTE_EVENTS) ?
in.unpackUuidNullable() : null;
Review Comment:
There are 4 request handlers (`ClientComputeExecuteColocatedRequest`,
`ClientComputeExecuteMapReduceRequest`,
`ClientComputeExecutePartitionedRequest`, `ClientComputeExecuteRequest`), but
here we update only 2 of them. Should we update the others too?
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
