korlov42 commented on code in PR #4929:
URL: https://github.com/apache/ignite-3/pull/4929#discussion_r1895575901
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteAbstractTransactionImpl.java:
##########
@@ -59,8 +63,15 @@ public abstract class IgniteAbstractTransactionImpl
implements InternalTransacti
* @param coordinatorId Transaction coordinator inconsistent ID.
* @param implicit True for an implicit transaction, false for an ordinary
one.
*/
- public IgniteAbstractTransactionImpl(TxManager txManager, UUID id, UUID
coordinatorId, boolean implicit) {
+ public IgniteAbstractTransactionImpl(
+ TxManager txManager,
+ ObservableTimestampProvider observableTsTracker,
Review Comment:
let's update javadoc
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlCursorCloseRequest.java:
##########
@@ -34,23 +33,19 @@ public class ClientSqlCursorCloseRequest {
* @param in Unpacker.
* @param out Packer.
* @param resources Resources.
- * @param transactions Transactional facade. Used to acquire last observed
time to propagate to client in response.
* @return Future representing result of operation.
*/
public static CompletableFuture<Void> process(
ClientMessageUnpacker in,
ClientMessagePacker out,
- ClientResourceRegistry resources,
- IgniteTransactionsImpl transactions
+ ClientResourceRegistry resources
) throws IgniteInternalCheckedException {
long resourceId = in.unpackLong();
ClientSqlResultSet asyncResultSet =
resources.remove(resourceId).get(ClientSqlResultSet.class);
return asyncResultSet.closeAsync()
.thenApply(ignored -> {
Review Comment:
you can remove `thenApply` entirely
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/ObservableTimestampProvider.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.tx;
+
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Interface is used to provide an observable timestamp into a transaction
operation.
+ */
+public interface ObservableTimestampProvider {
Review Comment:
it's better to name it `HybridTimestampTracker` and rename implementation to
`HybridTimestampTrackerImpl` so w will avoid hundreds of meaningless changes.
Besides, I believe `HybridTimestampTracker` is better fit for such interface
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -82,42 +83,48 @@ public class JdbcQueryEventHandlerImpl extends
JdbcHandlerBase implements JdbcQu
/** {@link SqlQueryType}s allowed in JDBC update statements. **/
private static final Set<SqlQueryType> UPDATE_STATEMENT_QUERIES =
Set.of(DML, SqlQueryType.DDL, SqlQueryType.KILL);
+ /**
+ * Observation timestamp tracker.
+ * TODO: IGNITE-24053 Remove this after the issue will be fixed.
+ * */
+ private final HybridTimestampTracker timestampTracker = new
HybridTimestampTracker();
Review Comment:
I've checked the mentioned JIRA, and it doesn't seem correct. At the moment,
JDBC driver doesn't track time on client's side, and driver cannot connect to
more than one server. During connection, server issues `connectionId` for every
`java.sql.Connection` instance, and every request from the connection provides
`connectionId`.
Given that said, we need to create one tracker per `JdbcConnectionContext`
and use it for every request within this connection. Changes in JDBC-related
requests processing methods are not necessary since driver doesn't track
observable time
--
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]
