[GitHub] [ignite-3] vldpyatkov commented on a diff in pull request #1960: IGNITE-19326 Close partition safe time trackers

["vldpyatkov (via GitHub)" <gi...@apache.org>]

vldpyatkov commented on code in PR #1960:
URL: https://github.com/apache/ignite-3/pull/1960#discussion_r1172431294


##########
modules/core/src/main/java/org/apache/ignite/internal/util/PendingComparableValuesTracker.java:
##########
@@ -46,6 +51,13 @@
     /** Current value. */
     private volatile T current;
 
+    /** Prevents double closing. */
+    @SuppressWarnings("unused")
+    private volatile boolean closeGuard;

Review Comment:
   Better to use AtomicBoolean here.



##########
modules/core/src/main/java/org/apache/ignite/internal/util/TrackerClosedException.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.util;
+
+/**
+ * Exception that will be thrown when the {@link PendingComparableValuesTracker} is closed.
+ */
+public class TrackerClosedException extends RuntimeException {

Review Comment:
   Why doesn't the internal exception inherit IgniteInternalException (or some other more fit)?



##########
modules/core/src/main/java/org/apache/ignite/internal/util/TrackerClosedException.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.util;
+
+/**
+ * Exception that will be thrown when the {@link PendingComparableValuesTracker} is closed.
+ */
+public class TrackerClosedException extends RuntimeException {
+    private static final long serialVersionUID = -3685913884384983930L;

Review Comment:
   Why is the id required?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2389,4 +2421,10 @@ private Collection<UUID> collectTableIndexes(UUID tableId) {
 
         return indexIds;
     }
+
+    private static void closeTracker(@Nullable PendingComparableValuesTracker<?> tracker) {

Review Comment:
   This method looks ugly, because it can work with any ManuallyCloseable object, but receives only PendingComparableValuesTracker.
   In possible, you will be able to rename it to closeIfExist(ManuallyCloseable o) and better move to a utility class.



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org