You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/04/27 09:11:12 UTC
[GitHub] [ignite] zstan opened a new pull request #7743: Ignite 12938
zstan opened a new pull request #7743:
URL: https://github.com/apache/ignite/pull/7743
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #7743: IGNITE-12938
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #7743:
URL: https://github.com/apache/ignite/pull/7743#discussion_r421479374
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -199,46 +204,49 @@ private VisorValidateIndexesJobResult call0() {
for (String cacheName : cacheNames) {
DynamicCacheDescriptor desc = ignite.context().cache().cacheDescriptor(cacheName);
- if (desc == null) {
+ if (desc == null)
missingCaches.add(cacheName);
-
- continue;
- }
-
- grpIds.add(desc.groupId());
+ else
+ if (ignite.context().cache().cacheGroup(desc.groupId()).affinityNode())
+ grpIds.add(desc.groupId());
}
if (!missingCaches.isEmpty()) {
- StringBuilder strBuilder = new StringBuilder("The following caches do not exist: ");
+ String errStr = "The following caches do not exist: " + String.join(", ", missingCaches);
- for (String name : missingCaches)
- strBuilder.append(name).append(", ");
-
- strBuilder.delete(strBuilder.length() - 2, strBuilder.length());
-
- throw new IgniteException(strBuilder.toString());
+ throw new IgniteException(errStr);
}
}
else {
Collection<CacheGroupContext> groups = ignite.context().cache().cacheGroups();
for (CacheGroupContext grp : groups) {
- if (!grp.systemCache() && !grp.isLocal())
+ if (!grp.systemCache() && !grp.isLocal() && grp.affinityNode())
grpIds.add(grp.groupId());
}
}
- List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
- List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
- List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
- List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
+ return grpIds;
+ }
+
+ /**
+ *
+ */
+ private VisorValidateIndexesJobResult call0() {
+ Set<Integer> grpIds = collectGroupIds();
+
+ /** Update counters per partition per group. */
+ final Map<Integer, Map<Integer, Long>> partsWithCntrsPerGrp =
+ getUpdateCountersSnapshot(ignite, grpIds);
+
+ IdleVerifyUtility.IdleChecker idleChecker = new IdleVerifyUtility.IdleChecker(ignite, partsWithCntrsPerGrp);
Review comment:
We need call this check before all check operations, this near operation :
Map<Integer, IndexIntegrityCheckIssue> integrityCheckResults = integrityCheckIndexesPartitions(grpIds, idleChecker);
need to be wrapped in this check too, isn`t 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #7743: IGNITE-12938
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #7743:
URL: https://github.com/apache/ignite/pull/7743#discussion_r421464832
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/verify/VerifyBackupPartitionsTaskV2.java
##########
@@ -252,97 +250,68 @@ public VerifyBackupPartitionsJobV2(VisorIdleVerifyTaskArg arg) {
/** {@inheritDoc} */
@Override public Map<PartitionKeyV2, PartitionHashRecordV2> execute() throws IgniteException {
+ try {
+ ignite.context().cache().context().database().waitForCheckpoint("VerifyBackupPartitions");
+ }
+ catch (IgniteCheckedException e) {
+ throw new IgniteException(
+ "Failed to wait for checkpoint before executing verify backup partitions task", e);
+ }
Review comment:
yep, due to async cluster wide cp, some pages can already synced to storage while others would be still in mem, thus further code : IdleVerifyUtility#checkPartitionsPageCrcSum would return different results. This additional cp prevent from this.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [ignite] ascherbakoff commented on a change in pull request #7743: IGNITE-12938
Posted by GitBox <gi...@apache.org>.
ascherbakoff commented on a change in pull request #7743:
URL: https://github.com/apache/ignite/pull/7743#discussion_r420721541
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/verify/VerifyBackupPartitionsTaskV2.java
##########
@@ -252,97 +250,68 @@ public VerifyBackupPartitionsJobV2(VisorIdleVerifyTaskArg arg) {
/** {@inheritDoc} */
@Override public Map<PartitionKeyV2, PartitionHashRecordV2> execute() throws IgniteException {
+ try {
+ ignite.context().cache().context().database().waitForCheckpoint("VerifyBackupPartitions");
+ }
+ catch (IgniteCheckedException e) {
+ throw new IgniteException(
+ "Failed to wait for checkpoint before executing verify backup partitions task", e);
+ }
Review comment:
I don't see the need in checkpoint here, can you elaborate ?
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -674,22 +676,33 @@ private void printProgressIfNeeded(Supplier<String> msgSup) {
}
/**
- * @param ctx Context.
- * @param idx Index.
+ * @param cacheCtxWithIdx Cache context and appropriate index.
+ * @param idleChecker Idle check closure.
*/
- private Future<Map<String, ValidateIndexesPartitionResult>> processIndexAsync(GridCacheContext ctx, Index idx) {
+ private Future<Map<String, ValidateIndexesPartitionResult>> processIndexAsync(
+ T2<GridCacheContext, Index> cacheCtxWithIdx,
+ IgniteInClosure<Integer> idleChecker
+ ) {
return calcExecutor.submit(new Callable<Map<String, ValidateIndexesPartitionResult>>() {
- @Override public Map<String, ValidateIndexesPartitionResult> call() throws Exception {
- return processIndex(ctx, idx);
+ /** {@inheritDoc} */
+ @Override public Map<String, ValidateIndexesPartitionResult> call() {
+ return processIndex(cacheCtxWithIdx, idleChecker);
Review comment:
Unnecessary spaces.
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -199,46 +204,49 @@ private VisorValidateIndexesJobResult call0() {
for (String cacheName : cacheNames) {
DynamicCacheDescriptor desc = ignite.context().cache().cacheDescriptor(cacheName);
- if (desc == null) {
+ if (desc == null)
missingCaches.add(cacheName);
-
- continue;
- }
-
- grpIds.add(desc.groupId());
+ else
+ if (ignite.context().cache().cacheGroup(desc.groupId()).affinityNode())
+ grpIds.add(desc.groupId());
}
if (!missingCaches.isEmpty()) {
- StringBuilder strBuilder = new StringBuilder("The following caches do not exist: ");
+ String errStr = "The following caches do not exist: " + String.join(", ", missingCaches);
- for (String name : missingCaches)
- strBuilder.append(name).append(", ");
-
- strBuilder.delete(strBuilder.length() - 2, strBuilder.length());
-
- throw new IgniteException(strBuilder.toString());
+ throw new IgniteException(errStr);
}
}
else {
Collection<CacheGroupContext> groups = ignite.context().cache().cacheGroups();
for (CacheGroupContext grp : groups) {
- if (!grp.systemCache() && !grp.isLocal())
+ if (!grp.systemCache() && !grp.isLocal() && grp.affinityNode())
grpIds.add(grp.groupId());
}
}
- List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
- List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
- List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
- List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
+ return grpIds;
+ }
+
+ /**
+ *
+ */
+ private VisorValidateIndexesJobResult call0() {
+ Set<Integer> grpIds = collectGroupIds();
+
+ /** Update counters per partition per group. */
+ final Map<Integer, Map<Integer, Long>> partsWithCntrsPerGrp =
+ getUpdateCountersSnapshot(ignite, grpIds);
+
+ IdleVerifyUtility.IdleChecker idleChecker = new IdleVerifyUtility.IdleChecker(ignite, partsWithCntrsPerGrp);
Review comment:
Don't we already compare counters in Map<PartitionKey, ValidateIndexesPartitionResult> processPartition() ?
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -482,28 +476,28 @@ private IndexIntegrityCheckIssue integrityCheckIndexPartition(CacheGroupContext
}
/**
- * @param grpCtx Group context.
- * @param part Local partition.
+ * @param grpCtxWithPart Group context and partition id.
*/
private Future<Map<PartitionKey, ValidateIndexesPartitionResult>> processPartitionAsync(
- final CacheGroupContext grpCtx,
- final GridDhtLocalPartition part
+ final T2<CacheGroupContext, GridDhtLocalPartition> grpCtxWithPart
) {
return calcExecutor.submit(new Callable<Map<PartitionKey, ValidateIndexesPartitionResult>>() {
- @Override public Map<PartitionKey, ValidateIndexesPartitionResult> call() throws Exception {
- return processPartition(grpCtx, part);
+ @Override public Map<PartitionKey, ValidateIndexesPartitionResult> call() {
+ return processPartition(grpCtxWithPart);
}
});
}
/**
- * @param grpCtx Group context.
- * @param part Local partition.
+ * @param grpCtxWithPart Group context and partition id.
*/
private Map<PartitionKey, ValidateIndexesPartitionResult> processPartition(
- CacheGroupContext grpCtx,
- GridDhtLocalPartition part
+ final T2<CacheGroupContext, GridDhtLocalPartition> grpCtxWithPart
Review comment:
Wasn't able to understand the purpose of the change. We have one more object as a result.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [ignite] asfgit closed pull request #7743: IGNITE-12938
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #7743:
URL: https://github.com/apache/ignite/pull/7743
----------------------------------------------------------------
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:
users@infra.apache.org