alex-plekhanov commented on a change in pull request #8822:
URL: https://github.com/apache/ignite/pull/8822#discussion_r588105977
##########
File path:
modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java
##########
@@ -2419,6 +2422,35 @@ protected void assertCountersSame(int partId, boolean
withReserveCntr) throws As
}
}
+ /**
+ * @param partId Partition.
+ * @param withReserveCntr {@code True} to compare reserve counters.
Because reserve counters are synced during
+ * @param cacheName Cache name.
+ * @param cnt Counter.
+ * @param reserved Reserved counter.
+ * PME invoking with {@code true} makes sense only after PME was finished.
+ */
+ protected void assertCountersAsExpected(int partId, boolean
withReserveCntr, String cacheName, long cnt,
+ long reserved) throws AssertionFailedError {
+ List<T3<String, @Nullable PartitionUpdateCounter, Boolean>> cntrMap =
G.allGrids().stream().filter(ignite ->
+ !ignite.configuration().isClientMode()).map(ignite ->
+ new T3<>(ignite.name(), counter(partId, cacheName, ignite.name()),
+
ignite.affinity(cacheName).isPrimary(ignite.cluster().localNode(),
partId))).collect(toList());
+
+ for (T3<String, PartitionUpdateCounter, Boolean> cntr : cntrMap) {
+ if (cntr.get2() == null)
+ continue;
+
+ assertEquals("Expecting same counters [partId=" + partId +
+ ", cntrs=" + cntrMap + ']', cnt, cntr.get2().get());
+
+ if (withReserveCntr)
Review comment:
Codestyle: `{}` for multiline `if`
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -1634,41 +1631,12 @@ private void distributedExchange() throws
IgniteCheckedException {
boolean skipWaitOnLocalJoin = localJoinExchange()
&& cctx.exchange().latch().canSkipJoiningNodes(initialVersion());
- if (context().exchangeFreeSwitch() && isBaselineNodeFailed()) {
+ if (context().exchangeFreeSwitch() && isBaselineNodeFailed())
Review comment:
Codestyle: `{}` for multiline `if`
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -1634,41 +1631,12 @@ private void distributedExchange() throws
IgniteCheckedException {
boolean skipWaitOnLocalJoin = localJoinExchange()
&& cctx.exchange().latch().canSkipJoiningNodes(initialVersion());
- if (context().exchangeFreeSwitch() && isBaselineNodeFailed()) {
+ if (context().exchangeFreeSwitch() && isBaselineNodeFailed())
// Currently MVCC does not support operations on partially
switched cluster.
if (cctx.kernalContext().coordinators().mvccEnabled())
- waitPartitionRelease(EXCHANGE_FREE_LATCH_ID, true, false,
null);
- else {
- boolean partitionedRecoveryRequired =
rebalancedInfo.primaryNodes.contains(firstDiscoEvt.eventNode());
-
- IgnitePredicate<IgniteInternalTx> replicatedOnly = tx -> {
- Collection<IgniteTxEntry> entries = tx.writeEntries();
- for (IgniteTxEntry entry : entries)
- if (cctx.cacheContext(entry.cacheId()).isReplicated())
- return true;
-
- // Checks only affected nodes contain replicated-free txs
with failed primaries.
- assert partitionedRecoveryRequired;
-
- return false;
- };
-
- // Assuming that replicated transactions are absent,
non-affected nodes will wait only this short sync.
- waitPartitionRelease(EXCHANGE_FREE_LATCH_ID + "-replicated",
true, false, replicatedOnly);
-
- String partitionedLatchId = EXCHANGE_FREE_LATCH_ID +
"-partitioned";
-
- if (partitionedRecoveryRequired)
- // This node contain backup partitions for failed
partitioned caches primaries. Waiting for recovery.
- waitPartitionRelease(partitionedLatchId, true, false,
null);
- else {
- // This node contain no backup partitions for failed
partitioned caches primaries. Recovery is not needed.
- Latch releaseLatch =
cctx.exchange().latch().getOrCreate(partitionedLatchId, initialVersion());
-
- releaseLatch.countDown(); // Await-free confirmation.
- }
- }
- }
+ waitPartitionRelease(EXCHANGE_FREE_LATCH_ID, true, false);
+ else
+ waitPartitionRelease(null, false, false);
Review comment:
Let's use some value for `latchId` (EXCHANGE_FREE_LATCH_ID). Even if
there is no distributed latch created, latch id is stored to timeBag: `Wait
partitions release latch [latch=...]`
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java
##########
@@ -918,11 +917,9 @@ public GridNearTxLocal newTx(
*
* @param topVer Topology version.
* @param node Failed node.
- * @param filter Recovery filter.
* @return Future that will be completed when all affected transactions
are recovered.
*/
- public IgniteInternalFuture<Boolean>
recoverLocalTxs(AffinityTopologyVersion topVer, ClusterNode node,
- IgnitePredicate<IgniteInternalTx> filter) {
+ public IgniteInternalFuture<Boolean>
recoverLocalTxs(AffinityTopologyVersion topVer, ClusterNode node) {
Review comment:
Redundant NL
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgnitionEx.java
##########
@@ -1121,7 +1121,16 @@ private static Ignite startConfigurations(
}
if (old != null)
- if (failIfStarted) {
+ if (old.grid() == null) { // Stopped but not removed from map yet.
Review comment:
This also can happen when grid with the same name is starting
concurrently, and in this case you will have 2 grid instances with the same
name with access from Ignition to only one of them. It's potentially dangerous
and I think much worse the previous behavior, when someone concurrently trying
to start grid with the same instance name as concurrently stopped grid (all you
need in this case is just wait for stop() method without concurrent 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.
For queries about this service, please contact Infrastructure at:
[email protected]
