PakhomovAlexander commented on code in PR #1865:
URL: https://github.com/apache/ignite-3/pull/1865#discussion_r1154207546


##########
modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/DeploymentManagerImpl.java:
##########
@@ -121,36 +131,42 @@ public CompletableFuture<Boolean> deployAsync(String id, 
Version version, Deploy
             return CompletableFuture.failedFuture(new 
DeploymentUnitReadException(e));
         }
 
-        CompletableFuture<Boolean> result = metastore.putIfNotExist(id, 
version, meta)
+        return metastore.putIfNotExist(id, version, meta)
                 .thenCompose(success -> {
                     if (success) {
-                        return deployer.deploy(id, version.render(), 
deploymentUnit.name(), unitContent);
-                    }
-                    LOG.error("Failed to deploy meta of unit " + id + ":" + 
version);
-                    return CompletableFuture.failedFuture(
-                            new DeploymentUnitAlreadyExistsException(id,
-                                    "Unit " + id + ":" + version + " already 
exists"));
-                })
-                .thenCompose(deployed -> {
-                    if (deployed) {
-                        return metastore.updateMeta(id, version,
-                                meta1 -> 
meta1.addConsistentId(clusterService.topologyService().localMember().name()));
-                    }
-                    return CompletableFuture.completedFuture(false);
-                })
-                .thenApply(completed -> {
-                    if (completed) {
-                        messaging.startDeployAsyncToCmg(id, version, 
deploymentUnit.name(), unitContent)
-                                .thenAccept(ids -> metastore.updateMeta(id, 
version, unitMeta -> {
-                                    for (String consistentId : ids) {
-                                        unitMeta.addConsistentId(consistentId);
+                        return tracker.track(id, version, deployer.deploy(id, 
version.render(), deploymentUnit.name(), unitContent)
+                                .thenCompose(deployed -> {
+                                    if (deployed) {
+                                        return metastore.updateMeta(id, 
version,
+                                                unitMeta -> unitMeta
+                                                        
.addConsistentId(clusterService.topologyService().localMember().name()));
+                                    }
+                                    return 
CompletableFuture.completedFuture(false);
+                                })
+                                .thenApply(completed -> {
+                                    if (completed) {
+                                        messaging.startDeployAsyncToCmg(id, 
version, deploymentUnit.name(), unitContent)
+                                                .thenAccept(ids -> 
metastore.updateMeta(id, version, unitMeta -> {
+                                                    for (String consistentId : 
ids) {
+                                                        
unitMeta.addConsistentId(consistentId);
+                                                    }
+                                                    
unitMeta.updateStatus(DEPLOYED);
+                                                }));
                                     }
-                                    unitMeta.updateStatus(DEPLOYED);
+                                    return completed;
                                 }));
+                    } else {
+                        if (force) {
+                            return undeployAsync(id, version)
+                                    .thenCompose(v -> deployAsync(id, version, 
deploymentUnit));
+                        }
+                        LOG.error("Failed to deploy meta of unit " + id + ":" 
+ version);

Review Comment:
   Could you log the reason why it is failed? "Already exists" is the reason 
and it should be mentioned in the log. Also, maybe it is a `warn` level?



##########
modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/IgniteDeployment.java:
##########
@@ -35,7 +35,20 @@ public interface IgniteDeployment extends IgniteComponent {
      * @param deploymentUnit Unit content.
      * @return Future with success or not result.
      */
-    CompletableFuture<Boolean> deployAsync(String id, Version version, 
DeploymentUnit deploymentUnit);
+    default CompletableFuture<Boolean> deployAsync(String id, Version version, 
DeploymentUnit deploymentUnit) {
+        return deployAsync(id, version, false, deploymentUnit);
+    }
+
+    /**
+     * Deploy provided unit to current node.
+     * After deploy finished, this deployment unit will be place to CMG group 
asynchronously.
+     *
+     * @param id Unit identifier. Not empty and not null.
+     * @param version Unit version.
+     * @param deploymentUnit Unit content.

Review Comment:
   `force` param is forgotten



-- 
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]

Reply via email to