petrov-mg commented on code in PR #13686:
URL: https://github.com/apache/ignite/pull/13686#discussion_r4326798008
##########
modules/core/src/main/java/org/apache/ignite/internal/management/cache/VerifyBackupPartitionsTask.java:
##########
@@ -251,10 +247,14 @@ public
VerifyBackupPartitionsJob(CacheIdleVerifyCommandArg arg) {
continue;
}
- cancelFuts(i, partHashCalcFuts);
+ // Propagate the cancellation to the async partition hash
calculations.
+ cancel();
Review Comment:
We need also to wait for all started jobs to complete.
##########
modules/core/src/main/java/org/apache/ignite/internal/management/cache/VerifyBackupPartitionsTask.java:
##########
@@ -494,32 +483,43 @@ private Future<Map<PartitionKey, PartitionHashRecord>>
calculatePartitionHashAsy
if (!part.reserve())
return res;
+ if (!gctx.offheap().stoppageLock().enterBusy()) {
+ part.release();
+
+ return res;
+ }
+
+ boolean gwLocked = false;
+
try {
PartitionUpdateCounter updCntr =
part.dataStore().partUpdateCounter();
PartitionUpdateCounter updateCntrBefore = updCntr == null
? null : updCntr.copy();
+ ignite.context().gateway().readLock();
Review Comment:
Do we still need it now that we use the off-heap manager lock?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManager.java:
##########
@@ -410,6 +411,13 @@ public void storeEntries(GridDhtLocalPartition part,
Iterator<GridCacheEntryInfo
*/
public void preloadPartition(int pardId) throws IgniteCheckedException;
+ /**
+ * Provides the offheap manager's stoppage lock.
+ *
+ * @return The offheap manager's stoppage lock.
+ */
+ public GridSpinBusyLock stoppageLock();
Review Comment:
It would be better to keep this lock encapsulated and introduce enterBusy
and leaveBusy methods in IgniteCacheOffheapManager.
##########
modules/core/src/main/java/org/apache/ignite/internal/management/cache/VerifyBackupPartitionsTask.java:
##########
@@ -494,32 +483,43 @@ private Future<Map<PartitionKey, PartitionHashRecord>>
calculatePartitionHashAsy
if (!part.reserve())
return res;
+ if (!gctx.offheap().stoppageLock().enterBusy()) {
Review Comment:
We need to check `cancelled.getAsBoolean() || gctx.isPreparedToStop() ||
ignite.context().isStopping()` here and throw a cancelled exception if it is
`true`.
We need to acquire and release the off-heap manager lock using a try/finally
block. The lock should be held only during the partition hash calculation.
If we fail to acquire the lock, we should throw a cancelled exception.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/verify/IdleVerifyUtility.java:
##########
@@ -331,6 +335,11 @@ public static List<Integer> compareUpdateCounters(
);
}
+ /** */
+ public static IgniteCheckedException cancelledException() {
+ return new NodeStoppingException(CANCELED_MSG);
Review Comment:
Using NodeStoppingException seems misleading.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
