Re: [PR] [IGNITE-21404] Do not wrap SqlException into RuntimeException for PlannerHelper.optimize. [ignite-3]
korlov42 merged PR #3625: URL: https://github.com/apache/ignite-3/pull/3625 -- 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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591: URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580546725 ## modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTxResourcesVacuumTest.java: ## @@ -0,0 +1,559 @@ +/* + * 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.table; + +import static java.util.stream.Collectors.toSet; +import static org.apache.ignite.internal.SessionUtils.executeUpdate; +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_STORAGE_PROFILE; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.findTupleToBeHostedOnNode; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.partitionAssignment; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.partitionIdForTuple; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.table; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.tableId; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.txId; +import static org.apache.ignite.internal.table.ItTransactionTestUtils.waitAndGetPrimaryReplica; +import static org.apache.ignite.internal.table.NodeUtils.transferPrimary; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition; +import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.apache.ignite.internal.tx.TxState.COMMITTED; +import static org.apache.ignite.internal.tx.impl.ResourceVacuumManager.RESOURCE_VACUUM_INTERVAL_MILLISECONDS_PROPERTY; +import static org.apache.ignite.internal.util.IgniteUtils.shutdownAndAwaitTermination; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Iterator; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import org.apache.ignite.InitParametersBuilder; +import org.apache.ignite.internal.ClusterPerTestIntegrationTest; +import org.apache.ignite.internal.app.IgniteImpl; +import org.apache.ignite.internal.placementdriver.ReplicaMeta; +import org.apache.ignite.internal.replicator.TablePartitionId; +import org.apache.ignite.internal.testframework.SystemPropertiesExtension; +import org.apache.ignite.internal.testframework.WithSystemProperty; +import org.apache.ignite.internal.thread.IgniteThreadFactory; +import org.apache.ignite.internal.thread.ThreadOperation; +import org.apache.ignite.internal.tx.TransactionMeta; +import org.apache.ignite.internal.tx.impl.TxManagerImpl; +import org.apache.ignite.internal.tx.message.TxCleanupMessage; +import org.apache.ignite.internal.tx.storage.state.TxStateStorage; +import org.apache.ignite.table.RecordView; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Integration tests for tx recources vacuum. + */ +@ExtendWith(SystemPropertiesExtension.class) +@WithSystemProperty(key = RESOURCE_VACUUM_INTERVAL_MILLISECONDS_PROPERTY, value = "1000") Review Comment: Ok, I mean that it's definitely required to check scheduler based logic, but for some scenarios it's just not needed, thus it's possible to manually trigger vacuum() there. And I can't see such manual interactions in the test. -- 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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580544405
##
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTxResourcesVacuumTest.java:
##
@@ -0,0 +1,559 @@
+/*
+ * 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.table;
+
+import static java.util.stream.Collectors.toSet;
+import static org.apache.ignite.internal.SessionUtils.executeUpdate;
+import static
org.apache.ignite.internal.catalog.CatalogService.DEFAULT_STORAGE_PROFILE;
+import static
org.apache.ignite.internal.table.ItTransactionTestUtils.findTupleToBeHostedOnNode;
+import static
org.apache.ignite.internal.table.ItTransactionTestUtils.partitionAssignment;
+import static
org.apache.ignite.internal.table.ItTransactionTestUtils.partitionIdForTuple;
+import static org.apache.ignite.internal.table.ItTransactionTestUtils.table;
+import static org.apache.ignite.internal.table.ItTransactionTestUtils.tableId;
+import static org.apache.ignite.internal.table.ItTransactionTestUtils.txId;
+import static
org.apache.ignite.internal.table.ItTransactionTestUtils.waitAndGetPrimaryReplica;
+import static org.apache.ignite.internal.table.NodeUtils.transferPrimary;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.apache.ignite.internal.tx.TxState.COMMITTED;
+import static
org.apache.ignite.internal.tx.impl.ResourceVacuumManager.RESOURCE_VACUUM_INTERVAL_MILLISECONDS_PROPERTY;
+import static
org.apache.ignite.internal.util.IgniteUtils.shutdownAndAwaitTermination;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Iterator;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import org.apache.ignite.InitParametersBuilder;
+import org.apache.ignite.internal.ClusterPerTestIntegrationTest;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.placementdriver.ReplicaMeta;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.testframework.SystemPropertiesExtension;
+import org.apache.ignite.internal.testframework.WithSystemProperty;
+import org.apache.ignite.internal.thread.IgniteThreadFactory;
+import org.apache.ignite.internal.thread.ThreadOperation;
+import org.apache.ignite.internal.tx.TransactionMeta;
+import org.apache.ignite.internal.tx.impl.TxManagerImpl;
+import org.apache.ignite.internal.tx.message.TxCleanupMessage;
+import org.apache.ignite.internal.tx.storage.state.TxStateStorage;
+import org.apache.ignite.table.RecordView;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Integration tests for tx recources vacuum.
+ */
+@ExtendWith(SystemPropertiesExtension.class)
+@WithSystemProperty(key = RESOURCE_VACUUM_INTERVAL_MILLISECONDS_PROPERTY,
value = "1000")
+public class ItTxResourcesVacuumTest extends ClusterPerTestIntegrationTest {
+/** Table name. */
+private static final String TABLE_NAME = "test_table";
+
+private static final Tuple INITIAL_TUPLE = Tuple.create().set("key",
1L).set("val", "1");
+
+private static final Function NEXT_TUPLE = t ->
Tuple.create()
+.set("key", t.longValue("key") + 1)
+.set("val", "" + (t.longValue
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580520929
##
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTransactionTestUtils.java:
##
@@ -0,0 +1,204 @@
+/*
+ * 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.table;
+
+import static java.util.Objects.requireNonNull;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toSet;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteTransaction;
+import static
org.apache.ignite.internal.TestWrappers.unwrapRecordBinaryViewImpl;
+import static org.apache.ignite.internal.TestWrappers.unwrapTableImpl;
+import static
org.apache.ignite.internal.distributionzones.rebalance.RebalanceUtil.stablePartAssignmentsKey;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+import org.apache.ignite.internal.affinity.Assignment;
+import org.apache.ignite.internal.affinity.Assignments;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.lang.ByteArray;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.MetaStorageManager;
+import org.apache.ignite.internal.placementdriver.ReplicaMeta;
+import org.apache.ignite.internal.replicator.ReplicationGroupId;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.tx.impl.ReadWriteTransactionImpl;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Test utils for transaction integration tests.
+ */
+public class ItTransactionTestUtils {
+/**
+ * Get the names of the nodes that are assignments of the given partition.
+ *
+ * @param node Any node in the cluster.
+ * @param grpId Group id.
+ * @return Node names.
+ */
+public static Set partitionAssignment(IgniteImpl node,
TablePartitionId grpId) {
Review Comment:
Test itself. Up to you.
--
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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580539171
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/PersistentTxStateVacuumizer.java:
##
@@ -0,0 +1,129 @@
+/*
+ * 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.impl;
+
+import static org.apache.ignite.internal.util.CompletableFutures.allOf;
+import static
org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+import static org.apache.ignite.internal.util.ExceptionUtils.unwrapCause;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.internal.hlc.ClockService;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.placementdriver.PlacementDriver;
+import org.apache.ignite.internal.replicator.ReplicaService;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import
org.apache.ignite.internal.replicator.exception.PrimaryReplicaMissException;
+import org.apache.ignite.internal.tx.message.TxMessagesFactory;
+import org.apache.ignite.internal.tx.message.VacuumTxStateReplicaRequest;
+import org.apache.ignite.network.ClusterNode;
+
+/**
+ * Implements the logic of persistent tx states vacuum.
+ */
+public class PersistentTxStateVacuumizer {
+private static final IgniteLogger LOG =
Loggers.forClass(PersistentTxStateVacuumizer.class);
+
+private static final TxMessagesFactory TX_MESSAGES_FACTORY = new
TxMessagesFactory();
+
+private final ReplicaService replicaService;
+
+private final ClusterNode localNode;
+
+private final ClockService clockService;
+
+private final PlacementDriver placementDriver;
+
+/**
+ * Constructor.
+ *
+ * @param replicaService Replica service.
+ * @param localNode Local node.
+ * @param clockService Clock service.
+ * @param placementDriver Placement driver.
+ */
+public PersistentTxStateVacuumizer(
+ReplicaService replicaService,
+ClusterNode localNode,
+ClockService clockService,
+PlacementDriver placementDriver
+) {
+this.replicaService = replicaService;
+this.localNode = localNode;
+this.clockService = clockService;
+this.placementDriver = placementDriver;
+}
+
+/**
+ * Vacuum persistent tx states.
+ *
+ * @param txIds Transaction ids to vacuum; map of commit partition ids to
sets of tx ids.
+ * @return A future, result is the set of successfully vacuumized txn
states.
+ */
+public CompletableFuture>
vacuumPersistentTxStates(Map> txIds) {
+Set successful = ConcurrentHashMap.newKeySet();
+List> futures = new ArrayList<>();
+HybridTimestamp now = clockService.now();
+
+txIds.forEach((commitPartitionId, txs) -> {
+CompletableFuture future =
placementDriver.getPrimaryReplica(commitPartitionId, now)
+.thenCompose(replicaMeta -> {
+// If the primary replica is absent this means that
another replica would become primary and
Review Comment:
1. If absent or doesn't match localNode.id()
2. Just "on primary replica election" ... cleanup completion timestamp
recalculated
--
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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580522883
##
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTransactionTestUtils.java:
##
@@ -0,0 +1,204 @@
+/*
+ * 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.table;
+
+import static java.util.Objects.requireNonNull;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toSet;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteTransaction;
+import static
org.apache.ignite.internal.TestWrappers.unwrapRecordBinaryViewImpl;
+import static org.apache.ignite.internal.TestWrappers.unwrapTableImpl;
+import static
org.apache.ignite.internal.distributionzones.rebalance.RebalanceUtil.stablePartAssignmentsKey;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+import org.apache.ignite.internal.affinity.Assignment;
+import org.apache.ignite.internal.affinity.Assignments;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.lang.ByteArray;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.MetaStorageManager;
+import org.apache.ignite.internal.placementdriver.ReplicaMeta;
+import org.apache.ignite.internal.replicator.ReplicationGroupId;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.tx.impl.ReadWriteTransactionImpl;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Test utils for transaction integration tests.
+ */
+public class ItTransactionTestUtils {
+/**
+ * Get the names of the nodes that are assignments of the given partition.
+ *
+ * @param node Any node in the cluster.
+ * @param grpId Group id.
+ * @return Node names.
+ */
+public static Set partitionAssignment(IgniteImpl node,
TablePartitionId grpId) {
+MetaStorageManager metaStorageManager = node.metaStorageManager();
+
+ByteArray stableAssignmentKey = stablePartAssignmentsKey(grpId);
+
+CompletableFuture assignmentEntryFut =
metaStorageManager.get(stableAssignmentKey);
+
+assertThat(assignmentEntryFut, willCompleteSuccessfully());
+
+Entry e = assignmentEntryFut.join();
+
+assertNotNull(e);
+assertFalse(e.empty());
+assertFalse(e.tombstone());
+
+Set a =
requireNonNull(Assignments.fromBytes(e.value())).nodes();
+
+return
a.stream().filter(Assignment::isPeer).map(Assignment::consistentId).collect(toSet());
+}
+
+/**
+ * Calculate the partition id on which the given tuple would be placed.
+ *
+ * @param node Any node in the cluster.
+ * @param tableName Table name.
+ * @param tuple Data tuple.
+ * @param tx Transaction, if present.
+ * @return Partition id.
+ */
+public static int partitionIdForTuple(IgniteImpl node, String tableName,
Tuple tuple, @Nullable Transaction tx) {
+TableImpl table = table(node, tableName);
+RecordBinaryViewImpl view =
unwrapRecordBinaryViewImpl(table.recordView());
+
+CompletableFuture rowFut = view.marshal(tx, tuple);
+assertThat(rowFut, willCompleteSuccessfully());
+BinaryRowEx row = rowFut.join();
+
+return table.internalTable().partitionId(row);
+}
+
+/**
+ * Generates some tuple that would be placed in the partition that is
hosted on the given node in the cluster.
+ *
+ * @param node Node that should host the result tuple.
+ * @param tableName Table name.
+ * @param tx Transaction, if present.
+ * @param initialTuple Initial tuple, for calculation.
+
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580520929
##
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTransactionTestUtils.java:
##
@@ -0,0 +1,204 @@
+/*
+ * 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.table;
+
+import static java.util.Objects.requireNonNull;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toSet;
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteTransaction;
+import static
org.apache.ignite.internal.TestWrappers.unwrapRecordBinaryViewImpl;
+import static org.apache.ignite.internal.TestWrappers.unwrapTableImpl;
+import static
org.apache.ignite.internal.distributionzones.rebalance.RebalanceUtil.stablePartAssignmentsKey;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+import org.apache.ignite.internal.affinity.Assignment;
+import org.apache.ignite.internal.affinity.Assignments;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.lang.ByteArray;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.MetaStorageManager;
+import org.apache.ignite.internal.placementdriver.ReplicaMeta;
+import org.apache.ignite.internal.replicator.ReplicationGroupId;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.tx.impl.ReadWriteTransactionImpl;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Test utils for transaction integration tests.
+ */
+public class ItTransactionTestUtils {
+/**
+ * Get the names of the nodes that are assignments of the given partition.
+ *
+ * @param node Any node in the cluster.
+ * @param grpId Group id.
+ * @return Node names.
+ */
+public static Set partitionAssignment(IgniteImpl node,
TablePartitionId grpId) {
Review Comment:
Test itself.
--
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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
sanpwc commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1580520355
##
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItTransactionPrimaryChangeTest.java:
##
@@ -87,7 +85,8 @@ protected void customizeInitParameters(InitParametersBuilder
builder) {
builder.clusterConfiguration("{"
+ " transaction: {"
-+ " implicitTransactionTimeout: 3"
++ " implicitTransactionTimeout: 3,"
++ " txnResourceTtl: 2"
Review Comment:
or 3, or 4, why it's 2?
--
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
Re: [PR] IGNITE-21938 Sql. Cover SQL F041-07 feature by tests [ignite-3]
zstan commented on code in PR #3642: URL: https://github.com/apache/ignite-3/pull/3642#discussion_r1580508874 ## modules/sql-engine/src/integrationTest/sql/join/inner/test_table_from_outer_join_used_in_inner.test: ## @@ -0,0 +1,55 @@ +# name: sql/join/inner/test_table_from_outer_join_used_in_inner.test +# description: SQL feature F041-07 (The inner table in a left or right outer join can also be used in an inner join) +# group: [Basic joined table] + +statement ok +PRAGMA enable_verification + +statement ok +CREATE TABLE t1(c11 INTEGER, c12 INTEGER, c13 CHAR); + +statement ok +INSERT INTO t1 VALUES (1, 2, 'a'), (2, 3, 'b'), (3, 4, 'c') + +statement ok +CREATE TABLE t2 (c21 INTEGER, c22 INTEGER, c23 CHAR); + +statement ok +INSERT INTO t2 VALUES (2, 3, 'a'), (3, 4, 'b'), (4, 3, 'c') + +query II rowsort +select j.c21, j.c22 from (SELECT c21, c22 from t2 LEFT OUTER JOIN t1 ON (t2.c21 = t1.c11)) j INNER JOIN t2 t22 ON (t22.c21 = j.c22); Review Comment: thanks, done -- 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
[PR] IGNITE-20294 Sql. Using UDF as a place for system_range function [ignite-3]
zstan opened a new pull request, #3666: URL: https://github.com/apache/ignite-3/pull/3666 Thank you for submitting the pull request. To streamline the review process of the patch and ensure better code quality we ask both an author and a reviewer to verify the following: ### The Review Checklist - [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following: \- There is a single JIRA ticket related to the pull request. \- The web-link to the pull request is attached to the JIRA ticket. \- The JIRA ticket has the Patch Available state. \- The description of the JIRA ticket explains WHAT was made, WHY and HOW. \- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE- Change summary where - number of JIRA issue. - [ ] **Design:** new code conforms with the design principles of the components it is added to. - [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable. - [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed. - [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources. ### Notes - [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide) -- 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
Re: [PR] Bump net.bytebuddy:byte-buddy from 1.14.12 to 1.14.14 [ignite-3]
ptupitsyn merged PR #3658: URL: https://github.com/apache/ignite-3/pull/3658 -- 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
Re: [PR] Bump org.openapi.generator from 7.4.0 to 7.5.0 [ignite-3]
ptupitsyn merged PR #3662: URL: https://github.com/apache/ignite-3/pull/3662 -- 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
Re: [PR] Bump bouncycastle from 1.76 to 1.78.1 [ignite-3]
ptupitsyn merged PR #3663: URL: https://github.com/apache/ignite-3/pull/3663 -- 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
Re: [I] IBinarizable : Mapping complex objects [ignite]
ptupitsyn closed issue #11325: IBinarizable : Mapping complex objects URL: https://github.com/apache/ignite/issues/11325 -- 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
Re: [I] IBinarizable : Mapping complex objects [ignite]
ptupitsyn commented on issue #11325: URL: https://github.com/apache/ignite/issues/11325#issuecomment-2078627474 Adding `address object` column to your DDL script fixes the issue. SQL engine does not drill down the nested object if there is no matching column. -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1580096527
##
modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/CatalogTestUtils.java:
##
@@ -91,11 +92,8 @@ public void beforeNodeStop() {
}
@Override
-public void stop() throws Exception {
-super.stop();
-
-clockWaiter.stop();
-metastore.stop();
+public CompletableFuture stopAsync() {
+return allOf(super.stopAsync(), clockWaiter.stopAsync(),
metastore.stopAsync());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1580096804
##
modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/CatalogTestUtils.java:
##
@@ -126,10 +124,8 @@ public void beforeNodeStop() {
}
@Override
-public void stop() throws Exception {
-super.stop();
-
-metastore.stop();
+public CompletableFuture stopAsync() {
+return allOf(super.stopAsync(), metastore.stopAsync());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1580096027
##
modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ItSimpleCounterServerTest.java:
##
@@ -97,14 +100,12 @@ void before() throws Exception {
server = new JraftServerImpl(service, workDir, raftConfiguration) {
@Override
-public synchronized void stop() throws Exception {
-super.stop();
-
-service.stop();
+public CompletableFuture stopAsync() {
+return CompletableFuture.allOf(super.stopAsync(),
service.stopAsync());
Review Comment:
done
--
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
Re: [I] IBinarizable : Mapping complex objects [ignite]
satishviswanathan commented on issue #11325: URL: https://github.com/apache/ignite/issues/11325#issuecomment-2078107652 @ptupitsyn Thank you that cleared by confusions. However this issue still persist, the Address fields are not updated. Although the data is not serialized to these fields, I can see the values are there in the cache. Is it because while creating the table using the ddl statement we have the VALUE_TYPE as Employee and the ignite server is unware of the "Address" type ? Attaching the sample code with the updated implementation [ignite-sample.zip](https://github.com/apache/ignite/files/15120307/ignite-sample.zip) -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579989020
##
modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java:
##
@@ -228,10 +230,16 @@ public void stop() throws Exception {
var ch = channel;
if (ch != null) {
-ch.close().await();
+try {
+ch.close().await();
Review Comment:
Not sure. I'd rather not touch this part in this PR, it has already grown
too big.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1580051040
##
modules/runner/src/main/java/org/apache/ignite/internal/app/LifecycleManager.java:
##
@@ -148,12 +153,11 @@ private synchronized void stopAllComponents() {
}
});
-new ReverseIterator<>(startedComponents).forEachRemaining(component ->
{
-try {
-component.stop();
-} catch (Exception e) {
-LOG.warn("Unable to stop component [component={},
nodeName={}]", e, component, nodeName);
-}
-});
+List> allComponentsStopFuture = new
ArrayList<>();
+
+new ReverseIterator<>(startedComponents).forEachRemaining(component ->
allComponentsStopFuture.add(component.stopAsync()));
+
+allOf(allComponentsStopFuture.toArray(CompletableFuture[]::new))
+.whenComplete((v, e) -> stopFuture.complete(null));
Review Comment:
The same applies to start then. I suggest we raise a separate JIRA to cover
that
##
modules/runner/src/main/java/org/apache/ignite/internal/app/LifecycleManager.java:
##
@@ -148,12 +153,11 @@ private synchronized void stopAllComponents() {
}
});
-new ReverseIterator<>(startedComponents).forEachRemaining(component ->
{
-try {
-component.stop();
-} catch (Exception e) {
-LOG.warn("Unable to stop component [component={},
nodeName={}]", e, component, nodeName);
-}
-});
+List> allComponentsStopFuture = new
ArrayList<>();
+
+new ReverseIterator<>(startedComponents).forEachRemaining(component ->
allComponentsStopFuture.add(component.stopAsync()));
+
+allOf(allComponentsStopFuture.toArray(CompletableFuture[]::new))
+.whenComplete((v, e) -> stopFuture.complete(null));
Review Comment:
I suggest we raise a separate JIRA to cover that
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579989020
##
modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java:
##
@@ -228,10 +230,16 @@ public void stop() throws Exception {
var ch = channel;
if (ch != null) {
-ch.close().await();
+try {
+ch.close().await();
Review Comment:
I'd rather not change the logic in this PR, it has already grown too big.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579986903
##
modules/code-deployment/src/test/java/org/apache/ignite/deployment/metastore/DeploymentUnitStoreImplTest.java:
##
@@ -102,12 +102,12 @@ public void setup() {
ClusterStatusWatchListener clusterListener = new
ClusterStatusWatchListener(clusterEventCallback);
metastore.registerClusterStatusListener(clusterListener);
-metaStorageManager.start();
+assertThat(metaStorageManager.startAsync(),
willCompleteSuccessfully());
toStop = () -> {
nodeListener.stop();
try {
-metaStorageManager.stop();
+assertThat(metaStorageManager.stopAsync(),
willCompleteSuccessfully());
} catch (Exception e) {
Review Comment:
Done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579984730
##
modules/core/src/main/java/org/apache/ignite/internal/manager/IgniteComponent.java:
##
@@ -47,7 +47,7 @@ default void beforeNodeStop() {
* Stops the component. It's guaranteed that during {@code
IgniteComponent#stop())} all components beneath given one are still running,
* however the node is no longer part of the topology and, accordingly,
network interaction is impossible.
*
- * @throws Exception If this component cannot be closed
+ * @return Future that will be completed when the asynchronous part of the
start is processed.
Review Comment:
Done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579982932
##
modules/compute/src/test/java/org/apache/ignite/internal/compute/ComputeComponentImplTest.java:
##
@@ -477,12 +478,12 @@ void remoteExecutionReleasesStopLock() throws Exception {
executeRemotely(SimpleJob.class.getName()).get();
-assertTimeoutPreemptively(Duration.ofSeconds(3), () ->
computeComponent.stop());
+assertTimeoutPreemptively(Duration.ofSeconds(3), () ->
assertThat(computeComponent.stopAsync(), willCompleteSuccessfully()));
}
@Test
void stoppedComponentReturnsExceptionOnExecuteRequestAttempt() throws
Exception {
-computeComponent.stop();
+computeComponent.stopAsync();
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579943253
##
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##
@@ -226,7 +226,10 @@ private void createAndStartComponents() {
lowWatermark
);
-assertThat(allOf(metaStorageManager.start(), catalogManager.start(),
indexManager.start()), willCompleteSuccessfully());
+assertThat(
+allOf(metaStorageManager.startAsync(),
catalogManager.startAsync(), indexManager.startAsync()),
Review Comment:
done
##
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##
@@ -1175,24 +1176,58 @@ public static byte[] byteBufferToByteArray(ByteBuffer
buffer) {
}
}
+private static CompletableFuture startAsync(Stream components) {
+return allOf(components
+.filter(Objects::nonNull)
+.map(IgniteComponent::startAsync)
+.toArray(CompletableFuture[]::new));
+}
+
+/**
+ * Asynchronously starts all ignite components.
+ *
+ * @param components Array of ignite components to start.
+ * @return CompletableFuture that will be completed when all components
are started.
+ */
+public static CompletableFuture startAsync(IgniteComponent...
components) {
+return startAsync(Stream.of(components));
+}
+
+/**
+ * Asynchronously starts all ignite components.
+ *
+ * @param components Collection of ignite components to start.
+ * @return CompletableFuture that will be completed when all components
are started.
+ */
+public static CompletableFuture startAsync(Collection components) {
+return startAsync(components.stream());
+}
+
+private static CompletableFuture stopAsync(Stream components) {
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579939781
##
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##
@@ -123,8 +123,8 @@ public void setUp() {
}
@AfterEach
-void tearDown() throws Exception {
-IgniteUtils.stopAll(metaStorageManager, catalogManager, indexManager);
+void tearDown() {
+assertThat(IgniteUtils.stopAsync(metaStorageManager, catalogManager,
indexManager), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579938244
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##
@@ -452,7 +452,13 @@ public synchronized void stop() throws Exception {
Collections.reverse(services);
-IgniteUtils.closeAll(services.stream().map(s -> s::stop));
+try {
+IgniteUtils.closeAll(services.stream().map(s -> s::stop));
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579935936
##
modules/client/src/test/java/org/apache/ignite/client/TestServer.java:
##
@@ -315,10 +317,11 @@ public FakePlacementDriver placementDriver() {
/** {@inheritDoc} */
@Override
public void close() throws Exception {
-module.stop();
-authenticationManager.stop();
-bootstrapFactory.stop();
-cfg.stop();
+assertThat(module.stopAsync(), willCompleteSuccessfully());
+assertThat(authenticationManager.stopAsync(),
willCompleteSuccessfully());
+assertThat(bootstrapFactory.stopAsync(), willCompleteSuccessfully());
+assertThat(cfg.stopAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579932468
##
modules/distribution-zones/src/test/java/org/apache/ignite/internal/distributionzones/BaseDistributionZoneManagerTest.java:
##
@@ -138,11 +134,11 @@ public void tearDown() throws Exception {
Collections.reverse(components);
IgniteUtils.closeAll(components.stream().map(c -> c::beforeNodeStop));
-IgniteUtils.closeAll(components.stream().map(c -> c::stop));
+assertThat(IgniteUtils.stopAsync(components),
willCompleteSuccessfully());
}
void startDistributionZoneManager() {
-assertThat(allOf(distributionZoneManager.start()),
willCompleteSuccessfully());
+assertThat(allOf(distributionZoneManager.startAsync()),
willCompleteSuccessfully());
Review Comment:
fixed
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629: URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579927727 ## modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageManagerImplTest.java: ## @@ -139,9 +139,9 @@ void setUp( metaStorageConfiguration ); -clusterService.start(); -raftManager.start(); -metaStorageManager.start(); +assertThat(clusterService.startAsync(), willCompleteSuccessfully()); Review Comment: done -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579922138
##
modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java:
##
@@ -201,8 +201,8 @@ private static class Node {
}
void start(PeersAndLearners configuration) {
-clusterService.start();
-raftManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
+assertThat(raftManager.startAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579915557
##
modules/network/src/testFixtures/java/org/apache/ignite/internal/network/utils/ClusterServiceTestUtils.java:
##
@@ -178,24 +177,12 @@ public CompletableFuture start() {
)
).join();
-bootstrapFactory.start();
-
-clusterSvc.start();
-
-return nullCompletedFuture();
+return allOf(bootstrapFactory.startAsync(),
clusterSvc.startAsync());
}
@Override
-public void stop() {
-try {
-IgniteUtils.closeAll(
-clusterSvc::stop,
-bootstrapFactory::stop,
-nodeConfigurationMgr::stop
-);
-} catch (Exception e) {
-throw new RuntimeException(e);
-}
+public CompletableFuture stopAsync() {
+return allOf(clusterSvc.stopAsync(),
bootstrapFactory.stopAsync(), nodeConfigurationMgr.stopAsync());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579912671
##
modules/placement-driver/src/integrationTest/java/org/apache/ignite/internal/placementdriver/Node.java:
##
@@ -52,13 +55,13 @@ class Node implements AutoCloseable {
}
CompletableFuture startAsync() {
-clusterService.start();
-loza.start();
-metastore.start();
+assertThat(clusterService.startAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579905344
##
modules/placement-driver/src/integrationTest/java/org/apache/ignite/internal/placementdriver/PlacementDriverManagerTest.java:
##
@@ -209,14 +209,14 @@ private void startPlacementDriverManager() {
new TestClockService(nodeClock)
);
-clusterService.start();
-anotherClusterService.start();
-raftManager.start();
-metaStorageManager.start();
+assertThat(clusterService.startAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579903175
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItLearnersTest.java:
##
@@ -120,8 +120,8 @@ Peer asPeer() {
}
void start() {
-clusterService.start();
-loza.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579901568
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItRaftGroupServiceTest.java:
##
@@ -221,8 +221,8 @@ String name() {
}
void start() {
-clusterService.start();
-loza.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579900229
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItRaftGroupServiceTest.java:
##
@@ -258,7 +258,10 @@ void beforeNodeStop() throws Exception {
}
void stop() throws Exception {
-IgniteUtils.closeAll(loza::stop, clusterService::stop);
+IgniteUtils.closeAll(
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629: URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579896240 ## modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/JraftAbstractTest.java: ## @@ -171,7 +173,7 @@ protected JraftServerImpl startServer(int idx, Consumer clo, Consume JraftServerImpl server = jraftServer(servers, idx, service, opts); -server.start(); +server.startAsync(); Review Comment: fixed -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579893056
##
modules/replicator/src/integrationTest/java/org/apache/ignite/internal/replicator/ItPlacementDriverReplicaSideTest.java:
##
@@ -195,19 +197,19 @@ public void beforeTest(TestInfo testInfo) {
replicaManagers.put(nodeName, replicaManager);
-clusterService.start();
-raftManager.start();
-replicaManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579892848
##
modules/replicator/src/integrationTest/java/org/apache/ignite/internal/replicator/ItPlacementDriverReplicaSideTest.java:
##
@@ -195,19 +197,19 @@ public void beforeTest(TestInfo testInfo) {
replicaManagers.put(nodeName, replicaManager);
-clusterService.start();
-raftManager.start();
-replicaManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
+assertThat(raftManager.startAsync(), willCompleteSuccessfully());
+assertThat(replicaManager.startAsync(),
willCompleteSuccessfully());
servicesToClose.add(() -> {
try {
replicaManager.beforeNodeStop();
raftManager.beforeNodeStop();
clusterService.beforeNodeStop();
-replicaManager.stop();
-raftManager.stop();
-clusterService.stop();
+assertThat(replicaManager.stopAsync(),
willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579887023
##
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildingManager.java:
##
@@ -142,20 +143,26 @@ public CompletableFuture start() {
}
@Override
-public void stop() throws Exception {
+public CompletableFuture stopAsync() {
if (!stopGuard.compareAndSet(false, true)) {
-return;
+return nullCompletedFuture();
}
busyLock.block();
-closeAllManually(
-indexBuilder,
-indexAvailabilityController,
-indexBuildController,
-changeIndexStatusTaskController
-);
+try {
+closeAllManually(
+indexBuilder,
+indexAvailabilityController,
+indexBuildController,
+changeIndexStatusTaskController
+);
+} catch (Exception e) {
+return failedFuture(e);
+}
shutdownAndAwaitTermination(executor, 10, TimeUnit.SECONDS);
Review Comment:
Done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579885185
##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerExceptionHandlingTest.java:
##
@@ -53,18 +53,18 @@ public class DdlCommandHandlerExceptionHandlingTest extends
IgniteAbstractTest {
void before() {
HybridClock clock = new HybridClockImpl();
catalogManager = createTestCatalogManager("test", clock);
-assertThat(catalogManager.start(), willCompleteSuccessfully());
+assertThat(catalogManager.startAsync(), willCompleteSuccessfully());
clockWaiter = new ClockWaiter("test", clock);
-assertThat(clockWaiter.start(), willCompleteSuccessfully());
+assertThat(clockWaiter.startAsync(), willCompleteSuccessfully());
commandHandler = new DdlCommandHandler(catalogManager, new
TestClockService(clock, clockWaiter), () -> 100);
}
@AfterEach
public void after() throws Exception {
-clockWaiter.stop();
-catalogManager.stop();
+assertThat(clockWaiter.stopAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579880642
##
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##
@@ -1180,16 +1181,24 @@ private RuntimeException handleStartException(Throwable
e) {
LOG.debug(errMsg, e);
-lifecycleManager.stopNode();
+try {
+lifecycleManager.stopNode().get();
+} catch (Exception ex) {
+IgniteException igniteException = new IgniteException(errMsg, ex);
+igniteException.addSuppressed(e);
+
+return igniteException;
+}
return new IgniteException(errMsg, e);
}
/**
* Stops ignite node.
*/
-public void stop() {
-lifecycleManager.stopNode();
+public void stop() throws ExecutionException, InterruptedException {
Review Comment:
Added
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579880267
##
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildingManager.java:
##
@@ -142,20 +143,26 @@ public CompletableFuture start() {
}
@Override
-public void stop() throws Exception {
+public CompletableFuture stopAsync() {
if (!stopGuard.compareAndSet(false, true)) {
-return;
+return nullCompletedFuture();
}
busyLock.block();
-closeAllManually(
-indexBuilder,
-indexAvailabilityController,
-indexBuildController,
-changeIndexStatusTaskController
-);
+try {
+closeAllManually(
+indexBuilder,
+indexAvailabilityController,
+indexBuildController,
+changeIndexStatusTaskController
+);
+} catch (Exception e) {
+return failedFuture(e);
+}
shutdownAndAwaitTermination(executor, 10, TimeUnit.SECONDS);
Review Comment:
I'd also include shutdownAndAwaitTermination(executor, ...) into
closeAllManually()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579870204
##
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##
@@ -1180,16 +1181,24 @@ private RuntimeException handleStartException(Throwable
e) {
LOG.debug(errMsg, e);
-lifecycleManager.stopNode();
+try {
+lifecycleManager.stopNode().get();
+} catch (Exception ex) {
+IgniteException igniteException = new IgniteException(errMsg, ex);
+igniteException.addSuppressed(e);
+
+return igniteException;
Review Comment:
Changed
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579867816
##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerExceptionHandlingTest.java:
##
@@ -53,18 +53,18 @@ public class DdlCommandHandlerExceptionHandlingTest extends
IgniteAbstractTest {
void before() {
HybridClock clock = new HybridClockImpl();
catalogManager = createTestCatalogManager("test", clock);
-assertThat(catalogManager.start(), willCompleteSuccessfully());
+assertThat(catalogManager.startAsync(), willCompleteSuccessfully());
clockWaiter = new ClockWaiter("test", clock);
-assertThat(clockWaiter.start(), willCompleteSuccessfully());
+assertThat(clockWaiter.startAsync(), willCompleteSuccessfully());
commandHandler = new DdlCommandHandler(catalogManager, new
TestClockService(clock, clockWaiter), () -> 100);
}
@AfterEach
public void after() throws Exception {
-clockWaiter.stop();
-catalogManager.stop();
+assertThat(clockWaiter.stopAsync(), willCompleteSuccessfully());
Review Comment:
IgniteUtils.stopAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579866759
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##
@@ -452,7 +452,13 @@ public synchronized void stop() throws Exception {
Collections.reverse(services);
-IgniteUtils.closeAll(services.stream().map(s -> s::stop));
+try {
+IgniteUtils.closeAll(services.stream().map(s -> s::stop));
Review Comment:
static import
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579864902
##
modules/runner/src/main/java/org/apache/ignite/internal/app/LifecycleManager.java:
##
@@ -148,12 +153,11 @@ private synchronized void stopAllComponents() {
}
});
-new ReverseIterator<>(startedComponents).forEachRemaining(component ->
{
-try {
-component.stop();
-} catch (Exception e) {
-LOG.warn("Unable to stop component [component={},
nodeName={}]", e, component, nodeName);
-}
-});
+List> allComponentsStopFuture = new
ArrayList<>();
+
+new ReverseIterator<>(startedComponents).forEachRemaining(component ->
allComponentsStopFuture.add(component.stopAsync()));
+
+allOf(allComponentsStopFuture.toArray(CompletableFuture[]::new))
+.whenComplete((v, e) -> stopFuture.complete(null));
Review Comment:
I'd say that we should explicitly specify pool to execute start/stop,
otherwise we may switch thread unpredictably.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579860866
##
modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/BaseCatalogManagerTest.java:
##
@@ -104,19 +102,16 @@ void setUp() {
() ->
CatalogManagerImpl.DEFAULT_PARTITION_IDLE_SAFE_TIME_PROPAGATION_PERIOD
);
-metastore.start();
-clockWaiter.start();
-manager.start();
+assertThat(metastore.startAsync(), willCompleteSuccessfully());
Review Comment:
done
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579859118
##
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##
@@ -1180,16 +1181,24 @@ private RuntimeException handleStartException(Throwable
e) {
LOG.debug(errMsg, e);
-lifecycleManager.stopNode();
+try {
+lifecycleManager.stopNode().get();
+} catch (Exception ex) {
+IgniteException igniteException = new IgniteException(errMsg, ex);
+igniteException.addSuppressed(e);
+
+return igniteException;
Review Comment:
I'd rather return original start exception here. May add stop one as
suppressed.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579859118
##
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##
@@ -1180,16 +1181,24 @@ private RuntimeException handleStartException(Throwable
e) {
LOG.debug(errMsg, e);
-lifecycleManager.stopNode();
+try {
+lifecycleManager.stopNode().get();
+} catch (Exception ex) {
+IgniteException igniteException = new IgniteException(errMsg, ex);
+igniteException.addSuppressed(e);
+
+return igniteException;
Review Comment:
I'd rather return original start exception here.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579857581
##
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##
@@ -1180,16 +1181,24 @@ private RuntimeException handleStartException(Throwable
e) {
LOG.debug(errMsg, e);
-lifecycleManager.stopNode();
+try {
+lifecycleManager.stopNode().get();
+} catch (Exception ex) {
+IgniteException igniteException = new IgniteException(errMsg, ex);
+igniteException.addSuppressed(e);
+
+return igniteException;
+}
return new IgniteException(errMsg, e);
}
/**
* Stops ignite node.
*/
-public void stop() {
-lifecycleManager.stopNode();
+public void stop() throws ExecutionException, InterruptedException {
Review Comment:
I'd say that we should provide both stopAsync() an stop() in order to be
consistent.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579857130
##
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerRecoveryTest.java:
##
@@ -81,8 +81,8 @@ public class CatalogManagerRecoveryTest extends
BaseIgniteAbstractTest {
private TestUpdateHandlerInterceptor interceptor;
@AfterEach
-void tearDown() throws Exception {
-IgniteUtils.stopAll(catalogManager, metaStorageManager);
+void tearDown() {
+assertThat(IgniteUtils.stopAsync(catalogManager, metaStorageManager),
willCompleteSuccessfully());
Review Comment:
Added exception handler in stopAsync
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579846730
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##
@@ -698,7 +698,9 @@ public CompletableFuture> dataNodes(long
causalityToken, int catalog
);
for (IgniteComponent component : otherComponents) {
-component.start();
+// TODO: Had to remove `willCompleteSuccessfully()` here
Review Comment:
@alievmirza
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579840663
##
modules/replicator/src/integrationTest/java/org/apache/ignite/internal/replicator/ItPlacementDriverReplicaSideTest.java:
##
@@ -195,19 +197,19 @@ public void beforeTest(TestInfo testInfo) {
replicaManagers.put(nodeName, replicaManager);
-clusterService.start();
-raftManager.start();
-replicaManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579841013
##
modules/replicator/src/integrationTest/java/org/apache/ignite/internal/replicator/ItPlacementDriverReplicaSideTest.java:
##
@@ -195,19 +197,19 @@ public void beforeTest(TestInfo testInfo) {
replicaManagers.put(nodeName, replicaManager);
-clusterService.start();
-raftManager.start();
-replicaManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
+assertThat(raftManager.startAsync(), willCompleteSuccessfully());
+assertThat(replicaManager.startAsync(),
willCompleteSuccessfully());
servicesToClose.add(() -> {
try {
replicaManager.beforeNodeStop();
raftManager.beforeNodeStop();
clusterService.beforeNodeStop();
-replicaManager.stop();
-raftManager.stop();
-clusterService.stop();
+assertThat(replicaManager.stopAsync(),
willCompleteSuccessfully());
Review Comment:
IgniteUtils.stopAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579840663
##
modules/replicator/src/integrationTest/java/org/apache/ignite/internal/replicator/ItPlacementDriverReplicaSideTest.java:
##
@@ -195,19 +197,19 @@ public void beforeTest(TestInfo testInfo) {
replicaManagers.put(nodeName, replicaManager);
-clusterService.start();
-raftManager.start();
-replicaManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
IgniteUtils.stopAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629: URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579824596 ## modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/JraftAbstractTest.java: ## @@ -171,7 +173,7 @@ protected JraftServerImpl startServer(int idx, Consumer clo, Consume JraftServerImpl server = jraftServer(servers, idx, service, opts); -server.start(); +server.startAsync(); Review Comment: assertion? -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579823834
##
modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ItSimpleCounterServerTest.java:
##
@@ -97,14 +100,12 @@ void before() throws Exception {
server = new JraftServerImpl(service, workDir, raftConfiguration) {
@Override
-public synchronized void stop() throws Exception {
-super.stop();
-
-service.stop();
+public CompletableFuture stopAsync() {
+return CompletableFuture.allOf(super.stopAsync(),
service.stopAsync());
Review Comment:
As usual IgniteUtils.stopAsyn()
--
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
[PR] IGNITE-22064 General MapReduce API [ignite-3]
valepakh opened a new pull request, #3665: URL: https://github.com/apache/ignite-3/pull/3665 https://issues.apache.org/jira/browse/IGNITE-22064 -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579729570
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItRaftGroupServiceTest.java:
##
@@ -258,7 +258,10 @@ void beforeNodeStop() throws Exception {
}
void stop() throws Exception {
-IgniteUtils.closeAll(loza::stop, clusterService::stop);
+IgniteUtils.closeAll(
Review Comment:
IgniteUtils.stopAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579727992
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItLearnersTest.java:
##
@@ -120,8 +120,8 @@ Peer asPeer() {
}
void start() {
-clusterService.start();
-loza.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579728997
##
modules/raft/src/integrationTest/java/org/apache/ignite/internal/raft/ItRaftGroupServiceTest.java:
##
@@ -221,8 +221,8 @@ String name() {
}
void start() {
-clusterService.start();
-loza.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579724921
##
modules/placement-driver/src/integrationTest/java/org/apache/ignite/internal/placementdriver/PlacementDriverManagerTest.java:
##
@@ -209,14 +209,14 @@ private void startPlacementDriverManager() {
new TestClockService(nodeClock)
);
-clusterService.start();
-anotherClusterService.start();
-raftManager.start();
-metaStorageManager.start();
+assertThat(clusterService.startAsync(), willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync()
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579724136
##
modules/placement-driver/src/integrationTest/java/org/apache/ignite/internal/placementdriver/Node.java:
##
@@ -52,13 +55,13 @@ class Node implements AutoCloseable {
}
CompletableFuture startAsync() {
-clusterService.start();
-loza.start();
-metastore.start();
+assertThat(clusterService.startAsync(), willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579723031
##
modules/network/src/testFixtures/java/org/apache/ignite/internal/network/utils/ClusterServiceTestUtils.java:
##
@@ -178,24 +177,12 @@ public CompletableFuture start() {
)
).join();
-bootstrapFactory.start();
-
-clusterSvc.start();
-
-return nullCompletedFuture();
+return allOf(bootstrapFactory.startAsync(),
clusterSvc.startAsync());
}
@Override
-public void stop() {
-try {
-IgniteUtils.closeAll(
-clusterSvc::stop,
-bootstrapFactory::stop,
-nodeConfigurationMgr::stop
-);
-} catch (Exception e) {
-throw new RuntimeException(e);
-}
+public CompletableFuture stopAsync() {
+return allOf(clusterSvc.stopAsync(),
bootstrapFactory.stopAsync(), nodeConfigurationMgr.stopAsync());
Review Comment:
It's not an equivalent of the original code. closeAll tolerates exceptions
while closing each resource and thus best-effort.
--
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
Re: [PR] IGNITE-21859 Causality token stays 0 for default zone [ignite-3]
korlov42 commented on code in PR #3653:
URL: https://github.com/apache/ignite-3/pull/3653#discussion_r1579707204
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/NewZoneEntry.java:
##
@@ -72,13 +72,17 @@ public CatalogEventParameters createEventParameters(long
causalityToken, int cat
public Catalog applyUpdate(Catalog catalog, long causalityToken) {
descriptor.updateToken(causalityToken);
+int defaultZoneId = catalog.defaultZone() != null
+? catalog.defaultZone().id()
+: descriptor.id();
Review Comment:
if default zone is not specified, the next created zone will be promoted as
default. This basically means that the first created zone will become default
one. Also, this will work as self-healing in case we lose default zone for some
reason (at the moment due to a some failure)
--
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
[PR] Bump io.github.bonede:tree-sitter-json from 0.20.1 to 0.21.0 [ignite-3]
dependabot[bot] opened a new pull request, #3664: URL: https://github.com/apache/ignite-3/pull/3664 Bumps [io.github.bonede:tree-sitter-json](https://github.com/bonede/tree-sitter-ng) from 0.20.1 to 0.21.0. Release notes Sourced from https://github.com/bonede/tree-sitter-ng/releases";>io.github.bonede:tree-sitter-json's releases. v0.21.0 Upgrade tree sitter to 0.21.0 v0.20.9 Upgrade tree sitter to 0.20.9 v0.20.8 No release notes provided. Commits See full diff in https://github.com/bonede/tree-sitter-ng/commits/v0.21.0";>compare view [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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
[PR] Bump bouncycastle from 1.76 to 1.78.1 [ignite-3]
dependabot[bot] opened a new pull request, #3663: URL: https://github.com/apache/ignite-3/pull/3663 Bumps `bouncycastle` from 1.76 to 1.78.1. Updates `org.bouncycastle:bcprov-jdk18on` from 1.76 to 1.78.1 Commits See full diff in https://github.com/bcgit/bc-java/commits";>compare view Updates `org.bouncycastle:bcpkix-jdk18on` from 1.76 to 1.78.1 Commits See full diff in https://github.com/bcgit/bc-java/commits";>compare view Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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
[PR] Bump org.openapi.generator from 7.4.0 to 7.5.0 [ignite-3]
dependabot[bot] opened a new pull request, #3662: URL: https://github.com/apache/ignite-3/pull/3662 Bumps org.openapi.generator from 7.4.0 to 7.5.0. [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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
[PR] Bump io.github.bonede:tree-sitter from 0.22.2 to 0.22.5 [ignite-3]
dependabot[bot] opened a new pull request, #3661: URL: https://github.com/apache/ignite-3/pull/3661 Bumps [io.github.bonede:tree-sitter](https://github.com/bonede/tree-sitter-ng) from 0.22.2 to 0.22.5. Release notes Sourced from https://github.com/bonede/tree-sitter-ng/releases";>io.github.bonede:tree-sitter's releases. v0.22.5 upgrade tree sitter to '0.22.5' Commits https://github.com/bonede/tree-sitter-ng/commit/e1a66018e36f4f71b2397784e7c65e88e4d9abc7";>e1a6601 readme https://github.com/bonede/tree-sitter-ng/commit/6b506ed6528260ae51db6ceba54c68b2b0ae6407";>6b506ed upgrade tree sitter to '0.22.5' https://github.com/bonede/tree-sitter-ng/commit/8153a381fd773280819da33d7d4668ce0101fa37";>8153a38 Merge pull request https://redirect.github.com/bonede/tree-sitter-ng/issues/12";>#12 from bonede/dependabot/github_actions/gradle/actions-3... https://github.com/bonede/tree-sitter-ng/commit/abd5c2b2303d9ac46bdfde0473661768de861965";>abd5c2b Bump gradle/actions from 3.2.0 to 3.3.0 https://github.com/bonede/tree-sitter-ng/commit/ca28d4c7643dcd96317e1ced749d86a3f7fafe84";>ca28d4c Merge pull request https://redirect.github.com/bonede/tree-sitter-ng/issues/10";>#10 from reneleonhardt/updates https://github.com/bonede/tree-sitter-ng/commit/c922e14d780af938aa33d6ea891e019442ac8491";>c922e14 chore(deps): Update dependencies https://github.com/bonede/tree-sitter-ng/commit/c39c82cc71777ae4f12e871d5324e48e1026cc68";>c39c82c upgrade tree sitter cpp to '0.20.5' See full diff in https://github.com/bonede/tree-sitter-ng/compare/v0.22.2...v0.22.5";>compare view [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579703433
##
modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java:
##
@@ -201,8 +201,8 @@ private static class Node {
}
void start(PeersAndLearners configuration) {
-clusterService.start();
-raftManager.start();
+assertThat(clusterService.startAsync(),
willCompleteSuccessfully());
+assertThat(raftManager.startAsync(), willCompleteSuccessfully());
Review Comment:
IgniteUtils.startAsync
--
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
Re: [PR] IGNITE-21859 Causality token stays 0 for default zone [ignite-3]
korlov42 commented on code in PR #3653:
URL: https://github.com/apache/ignite-3/pull/3653#discussion_r1579702498
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -363,6 +361,30 @@ public CompletableFuture compactCatalog(long
timestamp) {
return updateLog.saveSnapshot(new SnapshotEntry(catalog));
}
+private CompletableFuture createDefaultZone(Catalog emptyCatalog) {
+List createZoneEntries = CreateZoneCommand.builder()
+.zoneName(DEFAULT_ZONE_NAME)
+.partitions(DEFAULT_PARTITION_COUNT)
+.replicas(DEFAULT_REPLICA_COUNT)
+.dataNodesAutoAdjustScaleUp(IMMEDIATE_TIMER_VALUE)
+.dataNodesAutoAdjustScaleDown(INFINITE_TIMER_VALUE)
+.filter(DEFAULT_FILTER)
+.storageProfilesParams(
+
List.of(StorageProfileParams.builder().storageProfile(CatalogService.DEFAULT_STORAGE_PROFILE).build())
+)
+.build()
+.get(emptyCatalog);
+
+return updateLog.append(new VersionedUpdate(emptyCatalog.version() +
1, 0L, createZoneEntries))
+.handle((result, error) -> {
+if (error != null) {
+LOG.warn("Unable to create default zone.", error);
Review Comment:
in that case you will get validation error (see
[this](https://github.com/apache/ignite-3/pull/3653/files#diff-46682bbd0f65a3926b40f44aeb4322270a21764690a6aa8f556eb908827e5c18R114))
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579701159
##
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##
@@ -123,8 +123,8 @@ public void setUp() {
}
@AfterEach
-void tearDown() throws Exception {
-IgniteUtils.stopAll(metaStorageManager, catalogManager, indexManager);
+void tearDown() {
+assertThat(IgniteUtils.stopAsync(metaStorageManager, catalogManager,
indexManager), willCompleteSuccessfully());
Review Comment:
Here and there. Is it possible to use static imports for IgniteUtils or
there's clashing?
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629: URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579697076 ## modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageManagerImplTest.java: ## @@ -139,9 +139,9 @@ void setUp( metaStorageConfiguration ); -clusterService.start(); -raftManager.start(); -metaStorageManager.start(); +assertThat(clusterService.startAsync(), willCompleteSuccessfully()); Review Comment: startAsync -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629: URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579697076 ## modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageManagerImplTest.java: ## @@ -139,9 +139,9 @@ void setUp( metaStorageConfiguration ); -clusterService.start(); -raftManager.start(); -metaStorageManager.start(); +assertThat(clusterService.startAsync(), willCompleteSuccessfully()); Review Comment: IgniteUtils.startAsync(clusterService, raftManager, ...) -- 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
Re: [PR] IGNITE-22104 : Documentation of Custom Metrics [ignite]
NSAmelchev commented on code in PR #11331: URL: https://github.com/apache/ignite/pull/11331#discussion_r1579683015 ## docs/_docs/monitoring-metrics/custom-metrics.adoc: ## @@ -0,0 +1,67 @@ +// 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. += Custom Metrics + +WARNING: This feature is experimental and may change in future releases. + +Ignite provides various internal link:monitoring-metrics/new-metrics.adoc[metrics]. However, these metrics might +not be enough. User can design and publish own, custom metrics. Cusom Metrics base on +link:monitoring-metrics/new-metrics-system.adoc[Metric System]. + +== Custom metric creation. + +To register a custom metric, you need to add new link:monitoring-metrics/new-metrics-system#registry[registry] first. +After that, metrics can be added to this registry. + +=== Custom metric registry. + +You can create custom metric registries via `IgniteMetrics` interface which is obtained by `Ignite.metrics()`. + +`IgniteMetric` interface has only a couple of methods: + +* `MetricRegistry getOrCreate(String registryName)` provides new or existing custom metric registry. +* `void remove(String registryName)` removes entire custom metric registry. + + +=== Custom metric creation. + +To register new own metric, use `MetricRegistry` interface which is obtained by `IgniteMetrics.getOrCreate(...)`. + +`MetricRegistry` has several methods to put or remove metrics like: + +* `void register(String metricName, IntSupplier valueSupplier, @Nullable String description);` registers an integer-value metric. +* `void register(String metricName, DoubleSupplier valueSupplier, @Nullable String description);` registers a double-value metric. +* `void remove(String name);` removes metric. + + +== Naming convention. +Names of a custom metrics (and its registries) are similar to names of the internal metrics. The name can have dot-separated +parts like 'proces`s.status.suspended'. + +Prefix 'custom.' is always added to the custom registry name in `IgniteMetrics`. For instance, if the passed registry name is +'process.status.suspended', it is automatically extended to 'custom.process.status.suspended'. + + +== Limitations +* It is impossible to affect the internal metrics. +* Custom metrics are registered on-demand and aren't persistent. After node restarts, they have to be registered anew. Review Comment: ```suggestion * Custom metrics are registered on-demand and aren't persistent. After node restarts, they have to be registered a new. ``` -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579694364
##
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##
@@ -226,7 +226,10 @@ private void createAndStartComponents() {
lowWatermark
);
-assertThat(allOf(metaStorageManager.start(), catalogManager.start(),
indexManager.start()), willCompleteSuccessfully());
+assertThat(
+allOf(metaStorageManager.startAsync(),
catalogManager.startAsync(), indexManager.startAsync()),
Review Comment:
Here and there, should we use startAsync() that you've introduced in such
places?
--
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
Re: [PR] IGNITE-21859 Causality token stays 0 for default zone [ignite-3]
alievmirza commented on code in PR #3653:
URL: https://github.com/apache/ignite-3/pull/3653#discussion_r1579656780
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -363,6 +361,30 @@ public CompletableFuture compactCatalog(long
timestamp) {
return updateLog.saveSnapshot(new SnapshotEntry(catalog));
}
+private CompletableFuture createDefaultZone(Catalog emptyCatalog) {
+List createZoneEntries = CreateZoneCommand.builder()
+.zoneName(DEFAULT_ZONE_NAME)
+.partitions(DEFAULT_PARTITION_COUNT)
+.replicas(DEFAULT_REPLICA_COUNT)
+.dataNodesAutoAdjustScaleUp(IMMEDIATE_TIMER_VALUE)
+.dataNodesAutoAdjustScaleDown(INFINITE_TIMER_VALUE)
+.filter(DEFAULT_FILTER)
+.storageProfilesParams(
+
List.of(StorageProfileParams.builder().storageProfile(CatalogService.DEFAULT_STORAGE_PROFILE).build())
+)
+.build()
+.get(emptyCatalog);
+
+return updateLog.append(new VersionedUpdate(emptyCatalog.version() +
1, 0L, createZoneEntries))
+.handle((result, error) -> {
+if (error != null) {
+LOG.warn("Unable to create default zone.", error);
Review Comment:
What are the negative side effects if default zone wasn't created at all?
What will be the behaviour of a table creation without specified zone?
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/NewZoneEntry.java:
##
@@ -72,13 +72,17 @@ public CatalogEventParameters createEventParameters(long
causalityToken, int cat
public Catalog applyUpdate(Catalog catalog, long causalityToken) {
descriptor.updateToken(causalityToken);
+int defaultZoneId = catalog.defaultZone() != null
+? catalog.defaultZone().id()
+: descriptor.id();
Review Comment:
I do not understand why we take `descriptor.id()` of a new zone as a
defaultZoneId
--
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
Re: [PR] IGNITE-22104 : Documentation of Custom Metrics [ignite]
NSAmelchev commented on code in PR #11331: URL: https://github.com/apache/ignite/pull/11331#discussion_r1579687570 ## docs/_docs/monitoring-metrics/custom-metrics.adoc: ## @@ -0,0 +1,67 @@ +// 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. += Custom Metrics + +WARNING: This feature is experimental and may change in future releases. + +Ignite provides various internal link:monitoring-metrics/new-metrics.adoc[metrics]. However, these metrics might +not be enough. User can design and publish own, custom metrics. Cusom Metrics base on +link:monitoring-metrics/new-metrics-system.adoc[Metric System]. + +== Custom metric creation. + +To register a custom metric, you need to add new link:monitoring-metrics/new-metrics-system#registry[registry] first. +After that, metrics can be added to this registry. + +=== Custom metric registry. + +You can create custom metric registries via `IgniteMetrics` interface which is obtained by `Ignite.metrics()`. + +`IgniteMetric` interface has only a couple of methods: + +* `MetricRegistry getOrCreate(String registryName)` provides new or existing custom metric registry. +* `void remove(String registryName)` removes entire custom metric registry. + + +=== Custom metric creation. + +To register new own metric, use `MetricRegistry` interface which is obtained by `IgniteMetrics.getOrCreate(...)`. + +`MetricRegistry` has several methods to put or remove metrics like: + +* `void register(String metricName, IntSupplier valueSupplier, @Nullable String description);` registers an integer-value metric. +* `void register(String metricName, DoubleSupplier valueSupplier, @Nullable String description);` registers a double-value metric. +* `void remove(String name);` removes metric. + + +== Naming convention. +Names of a custom metrics (and its registries) are similar to names of the internal metrics. The name can have dot-separated +parts like 'proces`s.status.suspended'. + +Prefix 'custom.' is always added to the custom registry name in `IgniteMetrics`. For instance, if the passed registry name is +'process.status.suspended', it is automatically extended to 'custom.process.status.suspended'. + + +== Limitations +* It is impossible to affect the internal metrics. +* Custom metrics are registered on-demand and aren't persistent. After node restarts, they have to be registered anew. +* Configuration of a custom metric isn't supported. +* The names must not be empty, cannot have spaces or empty dot-separated parts. + + + Review Comment: Too many line breaks -- 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
Re: [PR] IGNITE-22104 : Documentation of Custom Metrics [ignite]
NSAmelchev commented on code in PR #11331: URL: https://github.com/apache/ignite/pull/11331#discussion_r1579683015 ## docs/_docs/monitoring-metrics/custom-metrics.adoc: ## @@ -0,0 +1,67 @@ +// 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. += Custom Metrics + +WARNING: This feature is experimental and may change in future releases. + +Ignite provides various internal link:monitoring-metrics/new-metrics.adoc[metrics]. However, these metrics might +not be enough. User can design and publish own, custom metrics. Cusom Metrics base on +link:monitoring-metrics/new-metrics-system.adoc[Metric System]. + +== Custom metric creation. + +To register a custom metric, you need to add new link:monitoring-metrics/new-metrics-system#registry[registry] first. +After that, metrics can be added to this registry. + +=== Custom metric registry. + +You can create custom metric registries via `IgniteMetrics` interface which is obtained by `Ignite.metrics()`. + +`IgniteMetric` interface has only a couple of methods: + +* `MetricRegistry getOrCreate(String registryName)` provides new or existing custom metric registry. +* `void remove(String registryName)` removes entire custom metric registry. + + +=== Custom metric creation. + +To register new own metric, use `MetricRegistry` interface which is obtained by `IgniteMetrics.getOrCreate(...)`. + +`MetricRegistry` has several methods to put or remove metrics like: + +* `void register(String metricName, IntSupplier valueSupplier, @Nullable String description);` registers an integer-value metric. +* `void register(String metricName, DoubleSupplier valueSupplier, @Nullable String description);` registers a double-value metric. +* `void remove(String name);` removes metric. + + +== Naming convention. +Names of a custom metrics (and its registries) are similar to names of the internal metrics. The name can have dot-separated +parts like 'proces`s.status.suspended'. + +Prefix 'custom.' is always added to the custom registry name in `IgniteMetrics`. For instance, if the passed registry name is +'process.status.suspended', it is automatically extended to 'custom.process.status.suspended'. + + +== Limitations +* It is impossible to affect the internal metrics. +* Custom metrics are registered on-demand and aren't persistent. After node restarts, they have to be registered anew. Review Comment: ```suggestion * Custom metrics are registered on-demand and aren't persistent. After node restarts, they have to be registered a new. ``` -- 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
Re: [PR] IGNITE-22104 : Documentation of Custom Metrics [ignite]
NSAmelchev commented on code in PR #11331: URL: https://github.com/apache/ignite/pull/11331#discussion_r1579679939 ## docs/_docs/monitoring-metrics/custom-metrics.adoc: ## @@ -0,0 +1,67 @@ +// 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. += Custom Metrics + +WARNING: This feature is experimental and may change in future releases. + +Ignite provides various internal link:monitoring-metrics/new-metrics.adoc[metrics]. However, these metrics might +not be enough. User can design and publish own, custom metrics. Cusom Metrics base on +link:monitoring-metrics/new-metrics-system.adoc[Metric System]. + +== Custom metric creation. + +To register a custom metric, you need to add new link:monitoring-metrics/new-metrics-system#registry[registry] first. +After that, metrics can be added to this registry. + +=== Custom metric registry. + +You can create custom metric registries via `IgniteMetrics` interface which is obtained by `Ignite.metrics()`. + +`IgniteMetric` interface has only a couple of methods: + +* `MetricRegistry getOrCreate(String registryName)` provides new or existing custom metric registry. +* `void remove(String registryName)` removes entire custom metric registry. + + +=== Custom metric creation. + +To register new own metric, use `MetricRegistry` interface which is obtained by `IgniteMetrics.getOrCreate(...)`. + +`MetricRegistry` has several methods to put or remove metrics like: + +* `void register(String metricName, IntSupplier valueSupplier, @Nullable String description);` registers an integer-value metric. +* `void register(String metricName, DoubleSupplier valueSupplier, @Nullable String description);` registers a double-value metric. +* `void remove(String name);` removes metric. + + +== Naming convention. +Names of a custom metrics (and its registries) are similar to names of the internal metrics. The name can have dot-separated +parts like 'proces`s.status.suspended'. Review Comment: redundant ` -- 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
Re: [PR] IGNITE-22104 : Documentation of Custom Metrics [ignite]
NSAmelchev commented on code in PR #11331: URL: https://github.com/apache/ignite/pull/11331#discussion_r1579675414 ## docs/_docs/monitoring-metrics/custom-metrics.adoc: ## @@ -0,0 +1,67 @@ +// 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. += Custom Metrics + +WARNING: This feature is experimental and may change in future releases. + +Ignite provides various internal link:monitoring-metrics/new-metrics.adoc[metrics]. However, these metrics might +not be enough. User can design and publish own, custom metrics. Cusom Metrics base on Review Comment: Cusom -> Custom -- 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
Re: [PR] Bump netty from 4.1.108.Final to 4.1.109.Final [ignite-3]
ptupitsyn merged PR #3657: URL: https://github.com/apache/ignite-3/pull/3657 -- 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
Re: [PR] Bump fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3 [ignite-3]
dependabot[bot] closed pull request #3648: Bump fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3 URL: https://github.com/apache/ignite-3/pull/3648 -- 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
Re: [PR] Bump fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3 [ignite-3]
dependabot[bot] commented on PR #3648: URL: https://github.com/apache/ignite-3/pull/3648#issuecomment-2077487393 OK, I won't notify you about version 11.x.x again, unless you re-open this PR. -- 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
Re: [PR] Bump fr.inria.gforge.spoon:spoon-core from 10.4.3-beta-20 to 11.0.1-beta-3 [ignite-3]
ptupitsyn commented on PR #3648: URL: https://github.com/apache/ignite-3/pull/3648#issuecomment-2077487178 Build failed. @dependabot ignore this major version -- 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
Re: [PR] Bump org.threeten:threetenbp from 1.6.8 to 1.6.9 [ignite-3]
ptupitsyn merged PR #3494: URL: https://github.com/apache/ignite-3/pull/3494 -- 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
Re: [PR] Bump com.github.johnrengelman.shadow from 7.1.2 to 8.1.1 [ignite-3]
ptupitsyn merged PR #3397: URL: https://github.com/apache/ignite-3/pull/3397 -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579654791
##
modules/distribution-zones/src/test/java/org/apache/ignite/internal/distributionzones/BaseDistributionZoneManagerTest.java:
##
@@ -138,11 +134,11 @@ public void tearDown() throws Exception {
Collections.reverse(components);
IgniteUtils.closeAll(components.stream().map(c -> c::beforeNodeStop));
-IgniteUtils.closeAll(components.stream().map(c -> c::stop));
+assertThat(IgniteUtils.stopAsync(components),
willCompleteSuccessfully());
}
void startDistributionZoneManager() {
-assertThat(allOf(distributionZoneManager.start()),
willCompleteSuccessfully());
+assertThat(allOf(distributionZoneManager.startAsync()),
willCompleteSuccessfully());
Review Comment:
allOf single))
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579648035
##
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItIgniteDistributionZoneManagerNodeRestartTest.java:
##
@@ -297,7 +301,8 @@ private PartialNode startPartialNode(int idx) {
);
for (IgniteComponent component : otherComponents) {
-component.start();
+// TODO: had to start without waiting as the test otherwise fails
with timeout.
Review Comment:
Please don't forget to remove todo, after receiving the feedback from
@alievmirza
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579640666
##
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java:
##
@@ -1175,24 +1176,58 @@ public static byte[] byteBufferToByteArray(ByteBuffer
buffer) {
}
}
+private static CompletableFuture startAsync(Stream components) {
+return allOf(components
+.filter(Objects::nonNull)
+.map(IgniteComponent::startAsync)
+.toArray(CompletableFuture[]::new));
+}
+
+/**
+ * Asynchronously starts all ignite components.
+ *
+ * @param components Array of ignite components to start.
+ * @return CompletableFuture that will be completed when all components
are started.
+ */
+public static CompletableFuture startAsync(IgniteComponent...
components) {
+return startAsync(Stream.of(components));
+}
+
+/**
+ * Asynchronously starts all ignite components.
+ *
+ * @param components Collection of ignite components to start.
+ * @return CompletableFuture that will be completed when all components
are started.
+ */
+public static CompletableFuture startAsync(Collection components) {
+return startAsync(components.stream());
+}
+
+private static CompletableFuture stopAsync(Stream components) {
Review Comment:
Please check my comment in
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerRecoveryTest.java
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579636368
##
modules/core/src/main/java/org/apache/ignite/internal/manager/IgniteComponent.java:
##
@@ -47,7 +47,7 @@ default void beforeNodeStop() {
* Stops the component. It's guaranteed that during {@code
IgniteComponent#stop())} all components beneath given one are still running,
* however the node is no longer part of the topology and, accordingly,
network interaction is impossible.
*
- * @throws Exception If this component cannot be closed
+ * @return Future that will be completed when the asynchronous part of the
start is processed.
Review Comment:
stop instead of start.
--
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
Re: [PR] IGNITE-21830 [ignite]
Vladsz83 commented on code in PR #11327:
URL: https://github.com/apache/ignite/pull/11327#discussion_r1579619834
##
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java:
##
@@ -7287,10 +7287,18 @@ private InetSocketAddress
checkConnection(List addrs, int tim
sock.connect(addr, perAddrTimeout);
liveAddrHolder.compareAndSet(null, addr);
+
+if (log.isInfoEnabled())
+log.info("Successful connection to the
server [address=" + addr + "]");
+}
+else if (log.isInfoEnabled()) {
+log.info("Connection to the server
[address=" + addr + "] is ignored. " +
+"Alive address is found at [address="
+ liveAddrHolder.get() + "]");
}
}
catch (Exception ignored) {
-// No-op.
+U.warn(log, "Failed to connect the socket to
the server [address=" + addr +
Review Comment:
If we lose the connection, we log `log.info('Previous node alive status
[alive=" + ok' + ...')`. Where `ok` is `false`. At INFO level. We except that
connection might be lost. Wy we use WARN here?
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579617708
##
modules/compute/src/test/java/org/apache/ignite/internal/compute/ComputeComponentImplTest.java:
##
@@ -477,12 +478,12 @@ void remoteExecutionReleasesStopLock() throws Exception {
executeRemotely(SimpleJob.class.getName()).get();
-assertTimeoutPreemptively(Duration.ofSeconds(3), () ->
computeComponent.stop());
+assertTimeoutPreemptively(Duration.ofSeconds(3), () ->
assertThat(computeComponent.stopAsync(), willCompleteSuccessfully()));
}
@Test
void stoppedComponentReturnsExceptionOnExecuteRequestAttempt() throws
Exception {
-computeComponent.stop();
+computeComponent.stopAsync();
Review Comment:
assertion is missing.
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1579605232
##
modules/code-deployment/src/test/java/org/apache/ignite/deployment/metastore/DeploymentUnitStoreImplTest.java:
##
@@ -102,12 +102,12 @@ public void setup() {
ClusterStatusWatchListener clusterListener = new
ClusterStatusWatchListener(clusterEventCallback);
metastore.registerClusterStatusListener(clusterListener);
-metaStorageManager.start();
+assertThat(metaStorageManager.startAsync(),
willCompleteSuccessfully());
toStop = () -> {
nodeListener.stop();
try {
-metaStorageManager.stop();
+assertThat(metaStorageManager.stopAsync(),
willCompleteSuccessfully());
} catch (Exception e) {
Review Comment:
Here and there, stopAsync no longer throws Exception, thus no need to catch
it.
--
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
Re: [PR] IGNITE-21830 [ignite]
Vladsz83 commented on code in PR #11327:
URL: https://github.com/apache/ignite/pull/11327#discussion_r1579604057
##
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java:
##
@@ -7287,10 +7287,18 @@ private InetSocketAddress
checkConnection(List addrs, int tim
sock.connect(addr, perAddrTimeout);
liveAddrHolder.compareAndSet(null, addr);
+
+if (log.isInfoEnabled())
+log.info("Successful connection to the
server [address=" + addr + "]");
+}
+else if (log.isInfoEnabled()) {
Review Comment:
I dont think we should use INFO level here. DEBUG is more suitable. My
message version:
`log.debug("Connection check to address ['" + addr + "] is ignored. Alive
address [" + liveAddrHolder.get() + "] is already found.") + "'."`
WDYT?
--
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
Re: [PR] IGNITE-21830 [ignite]
Vladsz83 commented on code in PR #11327:
URL: https://github.com/apache/ignite/pull/11327#discussion_r1579604057
##
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java:
##
@@ -7287,10 +7287,18 @@ private InetSocketAddress
checkConnection(List addrs, int tim
sock.connect(addr, perAddrTimeout);
liveAddrHolder.compareAndSet(null, addr);
+
+if (log.isInfoEnabled())
+log.info("Successful connection to the
server [address=" + addr + "]");
+}
+else if (log.isInfoEnabled()) {
Review Comment:
I dont think we should use INFO level here. DEBUG is more suitable. My
message version:
`log.debug("Connection check to ['" + addr + "] is ignored. Alive address ["
+ liveAddrHolder.get() + "] is already found.") + "'."`
WDYT?
--
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
Re: [PR] IGNITE-21830 [ignite]
Vladsz83 commented on code in PR #11327:
URL: https://github.com/apache/ignite/pull/11327#discussion_r1579570675
##
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java:
##
@@ -7287,10 +7287,18 @@ private InetSocketAddress
checkConnection(List addrs, int tim
sock.connect(addr, perAddrTimeout);
liveAddrHolder.compareAndSet(null, addr);
+
+if (log.isInfoEnabled())
+log.info("Successful connection to the
server [address=" + addr + "]");
+}
+else if (log.isInfoEnabled()) {
+log.info("Connection to the server
[address=" + addr + "] is ignored. " +
Review Comment:
Why we log here? We do not connect to a node. Just checking whether it is
alive. We duplicate fuhrter message:
`if (log.isInfoEnabled()) {
log.info("Connection check to previous
node done: [liveAddr=" + liveAddr
+ ", previousNode=" +
U.toShortString(previous) + ", addressesToCheck=" +
nodeAddrs + ", connectingNodeId=" +
nodeId + ']');
}`
--
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
