timoninmaxim commented on code in PR #10263:
URL: https://github.com/apache/ignite/pull/10263#discussion_r993381598
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -773,6 +815,112 @@ private IgniteInternalFuture<SnapshotOperationResponse>
initLocalSnapshotStartSt
"on the local node [missed=" + leftGrps + ", nodeId=" +
cctx.localNodeId() + ']'));
}
+ if (req.incremental()) {
+ SnapshotMetadata meta = readSnapshotMetadata(new File(
+ snapshotLocalDir(req.snapshotName(), req.snapshotPath()),
+
snapshotMetaFileName(cctx.localNode().consistentId().toString())
+ ));
+
+ try {
+ checkIncrementalCanBeCreated(req.snapshotName(),
req.snapshotPath(), meta);
+ }
+ catch (IgniteCheckedException e) {
+ return new GridFinishedFuture<>(e);
+ }
+
+ return initLocalIncrementalSnapshot(req, meta);
+ }
+ else
+ return initLocalFullSnapshot(req, grpIds, withMetaStorage);
+ }
+
+ /**
+ * @param req Request on snapshot creation.
+ * @param meta Full snapshot metadata.
+ * @return Future which will be completed when a snapshot has been started.
+ */
+ private IgniteInternalFuture<SnapshotOperationResponse>
initLocalIncrementalSnapshot(
+ SnapshotOperationRequest req,
+ SnapshotMetadata meta
+ ) {
+ IgniteInternalFuture<SnapshotOperationResponse> task0 =
registerTask(req.snapshotName(), new IncrementalSnapshotFutureTask(
+ cctx,
+ req.operationalNodeId(),
+ req.requestId(),
+ meta,
+ req.snapshotPath(),
+ req.incrementIndex(),
+ tmpWorkDir,
+ ioFactory
+ )).chain(fut -> {
+ if (fut.error() != null)
+ throw F.wrap(fut.error());
+
+ File incSnpDir = incrementalSnapshotLocalDir(req.snapshotName(),
req.snapshotPath(), req.incrementIndex());
+
+ assert incSnpDir.exists() : "Incremental snapshot directory must
exists";
+
+ IncrementalSnapshotMetadata incMeta = new
IncrementalSnapshotMetadata(
+ req.requestId(),
+ req.snapshotName(),
+ req.incrementIndex(),
+ cctx.localNode().consistentId().toString(),
+ pdsSettings.folderName(),
+ null /* WAL Pointer for CUT record goes here. */
+ );
+
+ writeSnapshotMetafile(
+ new File(incSnpDir,
incrementalSnapshotMetaFileName(req.incrementIndex())),
+ incMeta
+ );
+
+ return new SnapshotOperationResponse();
+ });
+
+ if (task0.isDone())
+ return task0;
+
+ if (log.isDebugEnabled()) {
+ log.debug("Incremental snapshot operation submited for execution" +
+ "[snpName=" + req.snapshotName() + ", incIdx=" +
req.incrementIndex());
+ }
+
+ clusterSnpReq = req;
+
+ cctx.kernalContext().pools().getSnapshotExecutorService().submit(() ->
{
+ SnapshotOperationRequest snpReq = clusterSnpReq;
+
+ AbstractSnapshotFutureTask<?> task =
locSnpTasks.get(snpReq.snapshotName());
+
+ if (task == null)
+ return;
+
+ if (log.isDebugEnabled()) {
+ log.debug("Incremental snapshot operation started" +
+ "[snpName=" + req.snapshotName() + ", incIdx=" +
req.incrementIndex());
+ }
+
+ writeSnapshotDirectoryToMetastorage(
+ incrementalSnapshotLocalDir(req.snapshotName(),
req.snapshotPath(), req.incrementIndex())
Review Comment:
Code duplication: `incrementalSnapshotLocalDir` also invoked within this
method - line 873.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheConfigurationEnrichment.java:
##########
@@ -93,4 +95,23 @@ public boolean hasField(String name) {
@Override public String toString() {
return S.toString(CacheConfigurationEnrichment.class, this);
}
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
Review Comment:
I suppose that some Ignite internal processes already check
`CacheConfiguration` for equality. For example, there are checks in
`ClusterCachesInfo#validateCacheGroupConfiguration`. Can we reuse it instead of
overriding the `equals` method? As the latter looks exceed - I suppose that
difference in the Enrichment fields shouldn't affect snapshot.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -1678,7 +1901,10 @@ public IgniteFutureImpl<Void> restoreSnapshot(String
name, @Nullable String snpP
for (File tmp : snapshotTmpDir().listFiles())
U.delete(tmp);
- deleteSnapshot(snpDir, pdsSettings.folderName());
+ if (INC_SNP_NAME_PATTERN.matcher(snpDir.getName()).matches() &&
snpDir.getAbsolutePath().contains(INC_SNP_DIR))
Review Comment:
As I can see, it's enough to check that `snpDir` contains `increments` path
segment, no need check name pattern actually.
##########
modules/core/src/test/resources/org.apache.ignite.util/GridCommandHandlerClusterByClassTest_help.output:
##########
@@ -140,12 +140,13 @@ This utility can do the following commands:
snapshot_name - Snapshot name.
Create cluster snapshot:
- control.(sh|bat) --snapshot create snapshot_name [--dest path] [--sync]
+ control.(sh|bat) --snapshot create snapshot_name [--dest path] [--sync]
[--incremental]
Parameters:
- snapshot_name - Snapshot name.
+ snapshot_name - Snapshot name. In case incremental snapshot
(--incremental) full snapshot name must be provided
Review Comment:
Here and below, dot is missed in the end of line
##########
modules/core/src/test/resources/org.apache.ignite.util/GridCommandHandlerClusterByClassTest_help.output:
##########
@@ -140,12 +140,13 @@ This utility can do the following commands:
snapshot_name - Snapshot name.
Create cluster snapshot:
- control.(sh|bat) --snapshot create snapshot_name [--dest path] [--sync]
+ control.(sh|bat) --snapshot create snapshot_name [--dest path] [--sync]
[--incremental]
Parameters:
- snapshot_name - Snapshot name.
+ snapshot_name - Snapshot name. In case incremental snapshot
(--incremental) full snapshot name must be provided
--dest path - Path to the directory where the snapshot will be saved.
If not specified, the default configured snapshot directory will be used.
--sync - Run the operation synchronously, the command will wait
for the entire operation to complete. Otherwise, it will be performed in the
background, and the command will immediately return control.
+ --incremental - Create an incremental snapshot for previously created
full snapshot. Full snapshot must be accessible via --dest or snapshot_name.
Review Comment:
> must be accessible via --dest or snapshot_name
OR, or AND (both dest and snapshot_name)?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -2178,6 +2433,123 @@ private IgniteFuture<Boolean>
executeRestoreManagementTask(
return new
IgniteFutureImpl<>(cctx.kernalContext().task().execute(taskCls, snpName));
}
+ /**
+ * Checks that incremental snapshot can be created for given full snapshot
and current cluster state.
+ *
+ * @param name Full snapshot name.
+ * @param snpPath Snapshot path.
+ * @param meta Full snapshot metadata.
+ */
+ private void checkIncrementalCanBeCreated(
+ String name,
+ @Nullable String snpPath,
+ SnapshotMetadata meta
+ ) throws IgniteCheckedException {
+ File snpDir = snapshotLocalDir(name, snpPath);
+
+ Set<String> aliveNodesConsIds = cctx.discovery().aliveServerNodes()
+ .stream()
+ .map(node -> node.consistentId().toString())
+ .collect(Collectors.toSet());
+
+ for (String consId : meta.baselineNodes()) {
+ if (!aliveNodesConsIds.contains(consId)) {
+ throw new IgniteCheckedException("Create incremental snapshot
request has been rejected. " +
+ "Node from full snapshot offline [consistentId=" + consId
+ ']');
+ }
+ }
+
+ File rootSnpCachesDir = new File(snpDir,
databaseRelativePath(meta.folderName()));
+
+ for (int grpId : meta.cacheGroupIds()) {
+ if (grpId == METASTORAGE_CACHE_ID)
+ continue;
+
+ CacheGroupContext gctx =
cctx.kernalContext().cache().cacheGroup(grpId);
+
+ if (gctx == null) {
+ throw new IgniteCheckedException("Create incremental snapshot
request has been rejected. " +
+ "Cache group destroyed [groupId=" + grpId + ']');
+ }
+
+ if (gctx.config().isEncryptionEnabled()) {
+ throw new IgniteCheckedException("Create incremental snapshot
request has been rejected. " +
+ "Encrypted cache groups not supported [groupId=" + grpId +
']');
+ }
+
+ List<File> snpCacheDir =
+ cacheDirectories(rootSnpCachesDir, grpName ->
gctx.cacheOrGroupName().equals(grpName));
+
+ if (snpCacheDir.isEmpty()) {
+ throw new IgniteCheckedException("Create incremental snapshot
request has been rejected. " +
+ "Cache group directory not found [groupId=" + grpId + ']');
+ }
+
+ assert snpCacheDir.size() == 1 : "Single snapshot cache directory
must be found";
+
+ JdkMarshaller marsh =
MarshallerUtils.jdkMarshaller(cctx.kernalContext().igniteInstanceName());
+
+ for (File snpDataFile :
FilePageStoreManager.cacheDataFiles(snpCacheDir.get(0))) {
+ StoredCacheData snpCacheData =
GridLocalConfigManager.readCacheData(
+ snpDataFile,
+ marsh,
+ cctx.kernalContext().config()
+ );
+
+ File nodeDataFile = new
File(snpDataFile.getAbsolutePath().replace(
+ rootSnpCachesDir.getAbsolutePath(),
+ pdsSettings.persistentStoreNodePath().getAbsolutePath()
+ ));
+
+ if (!nodeDataFile.exists()) {
+ throw new IgniteCheckedException("Create incremental
snapshot request has been rejected. " +
+ "Cache destroyed [cacheId=" + snpCacheData.cacheId() +
+ ", cacheName=" + snpCacheData.config().getName() +
']');
+ }
+
+ StoredCacheData nodeCacheData =
GridLocalConfigManager.readCacheData(
+ nodeDataFile,
+ marsh,
+ cctx.gridConfig()
+ );
+
+ addEncryptionInfoIfEnabled(nodeCacheData);
+
+ if (!snpCacheData.encryptedAwareEquals(
+ nodeCacheData,
+ cctx.kernalContext().config().getEncryptionSpi()
+ )) {
+ throw new IgniteCheckedException(
+ cacheChangedException(snpCacheData.cacheId(),
snpCacheData.config().getName())
+ );
+ }
+ }
+ }
+ }
+
+ /**
+ * Throw cache changed exception.
+ *
+ * @param cacheId Cache id.
+ * @param name Cache name.
+ */
+ public static String cacheChangedException(int cacheId, String name) {
+ return "Create incremental snapshot request has been rejected. " +
+ "Cache changed [cacheId=" + cacheId + ", cacheName=" + name + ']';
+ }
+
+ /** Adds {@link StoredCacheData#groupKeyEncrypted(GroupKeyEncrypted)} if
encryption enabled. */
+ private void addEncryptionInfoIfEnabled(StoredCacheData cacheData) throws
IgniteCheckedException {
Review Comment:
Should be unused anymore.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/wal/FileDescriptor.java:
##########
@@ -67,7 +65,7 @@ public FileDescriptor(File file, @Nullable Long idx) {
assert fileName.contains(WAL_SEGMENT_FILE_EXT);
- this.idx = idx == null ? Long.parseLong(fileName.substring(0,
WAL_SEGMENT_FILE_NAME_LENGTH)) : idx;
+ this.idx = idx == null ? Long.parseLong(fileName.substring(0,
NUMBER_FILE_NAME_LENGTH)) : idx;
Review Comment:
Let's move extraction of index to `IgniteUtils`, as the constant no more
part of this class.
--
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]
