timoninmaxim commented on code in PR #11368:
URL: https://github.com/apache/ignite/pull/11368#discussion_r1627348827
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotVerificationTask.java:
##########
@@ -100,19 +98,60 @@ public abstract class AbstractSnapshotVerificationTask
extends
/**
* @param name Snapshot name.
- * @param path Snapshot directory path.
- * @param incIdx Incremental snapshot index.
* @param constId Snapshot metadata file name.
- * @param groups Cache groups to be restored from the snapshot. May be
empty if all cache groups are being restored.
- * @param check If {@code true} check snapshot before restore.
+ * @param args Check snapshot parameters.
+ *
* @return Compute job.
*/
- protected abstract ComputeJob createJob(
- String name,
- @Nullable String path,
- int incIdx,
- String constId,
- Collection<String> groups,
- boolean check
- );
+ protected abstract AbstractSnapshotPartitionsVerifyJob createJob(String
name, String constId, SnapshotPartitionsVerifyTaskArg args);
Review Comment:
AbstractSnapshotVerificationJob, by analogue of the task name
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IncrementalSnapshotVerificationTask.java:
##########
@@ -198,10 +161,9 @@ public VerifyIncrementalSnapshotJob(
int incIdx,
String consId
) {
- this.snpName = snpName;
- this.snpPath = snpPath;
+ super(snpName, snpPath, consId, Collections.emptySet(), true);
Review Comment:
emptySet -> null
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotVerificationTask.java:
##########
@@ -100,19 +98,60 @@ public abstract class AbstractSnapshotVerificationTask
extends
/**
* @param name Snapshot name.
- * @param path Snapshot directory path.
- * @param incIdx Incremental snapshot index.
* @param constId Snapshot metadata file name.
- * @param groups Cache groups to be restored from the snapshot. May be
empty if all cache groups are being restored.
- * @param check If {@code true} check snapshot before restore.
+ * @param args Check snapshot parameters.
+ *
* @return Compute job.
*/
- protected abstract ComputeJob createJob(
- String name,
- @Nullable String path,
- int incIdx,
- String constId,
- Collection<String> groups,
- boolean check
- );
+ protected abstract AbstractSnapshotPartitionsVerifyJob createJob(String
name, String constId, SnapshotPartitionsVerifyTaskArg args);
Review Comment:
1. Misprint - constId -> consId
2. Javadocs says that it is "metadata filename", whether the docs or the
variable should be renamed
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotVerificationTask.java:
##########
@@ -100,19 +98,60 @@ public abstract class AbstractSnapshotVerificationTask
extends
/**
* @param name Snapshot name.
- * @param path Snapshot directory path.
- * @param incIdx Incremental snapshot index.
* @param constId Snapshot metadata file name.
- * @param groups Cache groups to be restored from the snapshot. May be
empty if all cache groups are being restored.
- * @param check If {@code true} check snapshot before restore.
+ * @param args Check snapshot parameters.
+ *
* @return Compute job.
*/
- protected abstract ComputeJob createJob(
- String name,
- @Nullable String path,
- int incIdx,
- String constId,
- Collection<String> groups,
- boolean check
- );
+ protected abstract AbstractSnapshotPartitionsVerifyJob createJob(String
name, String constId, SnapshotPartitionsVerifyTaskArg args);
+
+ /** */
+ protected abstract static class AbstractSnapshotPartitionsVerifyJob
extends ComputeJobAdapter {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** Ignite instance. */
+ @IgniteInstanceResource
+ protected IgniteEx ignite;
+
+ /** Injected logger. */
+ @LoggerResource
+ protected IgniteLogger log;
+
+ /** Snapshot name. */
+ protected final String snpName;
+
+ /** Snapshot directory path. */
+ @Nullable protected final String snpPath;
+
+ /** Consistent ID. */
+ protected final String consId;
+
+ /** Set of cache groups to be checked in the snapshot or {@code empty}
to check everything. */
Review Comment:
Can we simplify the logic? If {@code null} then check everything, otherwise
check the specified groups. I suppose Ignite already have check that the param
must not be empty.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotVerificationTask.java:
##########
@@ -100,19 +98,60 @@ public abstract class AbstractSnapshotVerificationTask
extends
/**
* @param name Snapshot name.
- * @param path Snapshot directory path.
- * @param incIdx Incremental snapshot index.
* @param constId Snapshot metadata file name.
- * @param groups Cache groups to be restored from the snapshot. May be
empty if all cache groups are being restored.
- * @param check If {@code true} check snapshot before restore.
+ * @param args Check snapshot parameters.
+ *
* @return Compute job.
*/
- protected abstract ComputeJob createJob(
- String name,
- @Nullable String path,
- int incIdx,
- String constId,
- Collection<String> groups,
- boolean check
- );
+ protected abstract AbstractSnapshotPartitionsVerifyJob createJob(String
name, String constId, SnapshotPartitionsVerifyTaskArg args);
+
+ /** */
+ protected abstract static class AbstractSnapshotPartitionsVerifyJob
extends ComputeJobAdapter {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** Ignite instance. */
+ @IgniteInstanceResource
+ protected IgniteEx ignite;
+
+ /** Injected logger. */
+ @LoggerResource
+ protected IgniteLogger log;
+
+ /** Snapshot name. */
+ protected final String snpName;
+
+ /** Snapshot directory path. */
+ @Nullable protected final String snpPath;
+
+ /** Consistent ID. */
+ protected final String consId;
+
+ /** Set of cache groups to be checked in the snapshot or {@code empty}
to check everything. */
+ protected final Collection<String> rqGrps;
+
+ /** If {@code true} check snapshot before restore. */
+ protected final boolean check;
+
+ /**
+ * @param snpName Snapshot name.
+ * @param snpPath Snapshot directory path.
+ * @param consId Consistent snapshot metadata file name.
+ * @param rqGrps Set of cache groups to be checked in the snapshot or
{@code empty} to check everything.
+ * @param check If {@code true} check snapshot before restore.
Review Comment:
AFAIU, this param controls amount of checks performed within the task. It is
used in `SnapshotHandlerContext` and has different javadoc. Let's unify docs
for this param?
--
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]
