sashapolo commented on code in PR #5574:
URL: https://github.com/apache/ignite-3/pull/5574#discussion_r2030668198


##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClockImpl.java:
##########
@@ -44,6 +49,14 @@ public class HybridClockImpl implements HybridClock {
 
     private final List<ClockUpdateListener> updateListeners = new 
CopyOnWriteArrayList<>();
 
+    public HybridClockImpl() {

Review Comment:
   Why do we need two constructors?



##########
modules/rocksdb-common/src/main/java/org/apache/ignite/internal/rocksdb/flush/RocksDbFlusher.java:
##########
@@ -185,7 +186,7 @@ public void removeColumnFamily(ColumnFamilyHandle handle) {
      *
      * @param schedule {@code true} if {@link RocksDB#flush(FlushOptions)} 
should be explicitly triggerred in the near future. Please refer
      *      to {@link RocksDbFlusher#RocksDbFlusher(String, 
IgniteSpinBusyLock, ScheduledExecutorService, ExecutorService, IntSupplier,
-     *      LogSyncer, Runnable)} parameters description to see what's really 
happening in this case.
+     *      LogSyncer, FailureProcessor, Runnable)} parameters description to 
see what's really happening in this case.

Review Comment:
   It would be easier to remove these parameters from the javadoc altogether



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/ZoneRebalanceRaftGroupEventsListener.java:
##########
@@ -203,7 +209,12 @@ public void onLeaderElected(long term) {
                     }
                 } catch (Exception e) {
                     // TODO: IGNITE-14693
-                    LOG.warn("Unable to start rebalance [tablePartitionId, 
term={}]", e, zonePartitionId, term);
+                    processCriticalFailure(
+                            failureProcessor,
+                            e,
+                            "Unable to start rebalance [tablePartitionId=%s, 
term=%s]",

Review Comment:
   ```suggestion
                               "Unable to start rebalance [zonePartitionId=%s, 
term=%s]",
   ```



##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/topology/LogicalTopologyImpl.java:
##########
@@ -53,15 +58,24 @@ public class LogicalTopologyImpl implements LogicalTopology 
{
 
     private final ClusterStateStorage storage;
 
+    private final FailureProcessor failureProcessor;
+
     private final ClusterStateStorageManager clusterStateStorageManager;
 
     private final List<LogicalTopologyEventListener> listeners = new 
CopyOnWriteArrayList<>();
 
     private volatile @Nullable UUID clusterId;
 
     /** Constructor. */
+    @TestOnly

Review Comment:
   Can this be avoided?



##########
modules/transactions/build.gradle:
##########
@@ -45,6 +45,7 @@ dependencies {
     implementation project(':ignite-workers')
     implementation project(':ignite-low-watermark')
     implementation project(':ignite-system-view-api')
+    implementation project(':ignite-failure-handler')

Review Comment:
   Why isn't it in the api section?



##########
modules/core/src/main/java/org/apache/ignite/internal/failure/FailureProcessorUtils.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.failure;
+
+import static org.apache.ignite.internal.failure.FailureType.CRITICAL_ERROR;
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import org.apache.ignite.internal.lang.IgniteInternalException;
+
+/**
+ * Utils making it easier to report failures with {@link FailureProcessor}.
+ */
+public class FailureProcessorUtils {
+    /**
+     * Reports a failure to a failure processor capturing the call site 
context.
+     *
+     * @param processor Processor used to process the failure.
+     * @param th The failure.
+     * @param messageFormat Message format (same as {@link 
String#format(String, Object...)} accepts).
+     * @param args Arguments for the message.
+     */
+    public static void processCriticalFailure(FailureProcessor processor, 
Throwable th, String messageFormat, Object... args) {

Review Comment:
   Why can't this be a method of the FailureProcessor?



-- 
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

Reply via email to