Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1608#discussion_r74027328
--- Diff: storm-core/src/jvm/org/apache/storm/StormSubmitter.java ---
@@ -238,39 +239,24 @@ public static void submitTopologyAs(String name, Map
stormConf, StormTopology to
}
// Dependency uploading only makes sense for distributed
mode
- List<String> jarsBlobKeys;
+ List<String> jarsBlobKeys = Collections.emptyList();
List<String> artifactsBlobKeys;
DependencyUploader uploader = new DependencyUploader();
try {
uploader.init();
jarsBlobKeys =
uploadDependencyJarsToBlobStore(uploader);
artifactsBlobKeys =
uploadDependencyArtifactsToBlobStore(uploader);
+ setDependencyBlobsToTopology(topology, jarsBlobKeys,
artifactsBlobKeys);
+ submitTopologyInDistributeMode(name, topology, opts,
progressListener, asUser, conf, serConf);
+ } catch (Throwable e) {
+ // remove uploaded jars blobs, not artifacts since
they're shared across the cluster
+ uploader.deleteBlobs(jarsBlobKeys);
--- End diff --
@satishd
In fact, submitTopologyWithOpts() can also throw AuthorizationException and
org.apache.thrift.TException, too.
Let's back to flow of logic.
Please note that we didn't catch Throwable (some known Exceptions and also
RuntimeException) and just pass to caller. And there's also no return for
Nimbus.Client.submitTopology(), which indicates that unless we get TException
or other Exceptions it should be treated as succeed. (If not we can't guarantee
topology deploy is succeed.)
I think this assumption is right for now, and assumption should be right
unless we add some operations after calling submitTopologyInDistributeMode() or
adding some operations after logging "Finished submitting topology".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
