nwangtw commented on a change in pull request #3710:
URL: https://github.com/apache/incubator-heron/pull/3710#discussion_r727736026
##########
File path:
heron/scheduler-core/src/java/org/apache/heron/scheduler/LaunchRunner.java
##########
@@ -169,13 +173,45 @@ public void call() throws LauncherException,
PackingException, SubmitDryRunRespo
"Failed to set execution state for topology '%s'", topologyName));
}
- // launch the topology, clear the state if it fails
- if (!launcher.launch(packedPlan)) {
+ // Launch the topology, clear the state if it fails. Some schedulers throw
exceptions instead of
+ // returning false. In some cases the scheduler needs to have the topology
deleted.
+ try {
+ if (!launcher.launch(packedPlan)) {
+ throw new TopologySubmissionException(null);
+ }
+ } catch (TopologySubmissionException e) {
+ // Compile error message to throw.
+ final StringBuilder errorMessage = new StringBuilder(
+ String.format("Failed to launch topology '%s'", topologyName));
+ if (e.getMessage() != null) {
+ errorMessage.append(String.format("%n%s", e.getMessage()));
Review comment:
"\n%s"? Maybe `append("\n"); append(e.getMessage());` is more efficient.
##########
File path:
heron/scheduler-core/src/java/org/apache/heron/scheduler/LaunchRunner.java
##########
@@ -169,13 +173,45 @@ public void call() throws LauncherException,
PackingException, SubmitDryRunRespo
"Failed to set execution state for topology '%s'", topologyName));
}
- // launch the topology, clear the state if it fails
- if (!launcher.launch(packedPlan)) {
+ // Launch the topology, clear the state if it fails. Some schedulers throw
exceptions instead of
+ // returning false. In some cases the scheduler needs to have the topology
deleted.
+ try {
+ if (!launcher.launch(packedPlan)) {
+ throw new TopologySubmissionException(null);
+ }
+ } catch (TopologySubmissionException e) {
+ // Compile error message to throw.
+ final StringBuilder errorMessage = new StringBuilder(
+ String.format("Failed to launch topology '%s'", topologyName));
+ if (e.getMessage() != null) {
+ errorMessage.append(String.format("%n%s", e.getMessage()));
+ }
+
+ try {
+ // Clear state from the Scheduler via RPC.
+ Scheduler.KillTopologyRequest killTopologyRequest =
Scheduler.KillTopologyRequest
+ .newBuilder()
+ .setTopologyName(topologyName).build();
+
+ ISchedulerClient schedulerClient = new SchedulerClientFactory(config,
runtime)
+ .getSchedulerClient();
+ if (!schedulerClient.killTopology(killTopologyRequest)) {
+ final String logMessage =
+ String.format("Failed to remove topology '%s' from scheduler
after failed submit. "
+ + "Please re-try the kill command.", topologyName);
+ errorMessage.append(String.format("%n%s", logMessage));
Review comment:
"\n%s"? Maybe `append("\n"); append(e.getMessage());` is more efficient.
##########
File path:
heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/KubernetesContext.java
##########
@@ -172,6 +179,15 @@ static String getContainerVolumeMountPath(Config config) {
return config.getStringValue(KUBERNETES_CONTAINER_VOLUME_MOUNT_PATH);
}
+ public static String getPodTemplateConfigMapName(Config config) {
+ return config.getStringValue(KUBERNETES_POD_TEMPLATE_CONFIGMAP_NAME);
+ }
+
+ public static boolean getPodTemplateConfigMapDisabled(Config config) {
+ final String disabled =
config.getStringValue(KUBERNETES_POD_TEMPLATE_CONFIGMAP_DISABLED);
+ return disabled != null &&
disabled.toLowerCase(Locale.ROOT).equals("true");
Review comment:
I am wondering if `equalsIgnoreCase()` is easier to maintain.
##########
File path:
heron/scheduler-core/src/java/org/apache/heron/scheduler/LaunchRunner.java
##########
@@ -169,13 +173,45 @@ public void call() throws LauncherException,
PackingException, SubmitDryRunRespo
"Failed to set execution state for topology '%s'", topologyName));
}
- // launch the topology, clear the state if it fails
- if (!launcher.launch(packedPlan)) {
+ // Launch the topology, clear the state if it fails. Some schedulers throw
exceptions instead of
+ // returning false. In some cases the scheduler needs to have the topology
deleted.
+ try {
+ if (!launcher.launch(packedPlan)) {
+ throw new TopologySubmissionException(null);
+ }
+ } catch (TopologySubmissionException e) {
+ // Compile error message to throw.
+ final StringBuilder errorMessage = new StringBuilder(
+ String.format("Failed to launch topology '%s'", topologyName));
+ if (e.getMessage() != null) {
+ errorMessage.append(String.format("%n%s", e.getMessage()));
+ }
+
+ try {
+ // Clear state from the Scheduler via RPC.
+ Scheduler.KillTopologyRequest killTopologyRequest =
Scheduler.KillTopologyRequest
+ .newBuilder()
+ .setTopologyName(topologyName).build();
+
+ ISchedulerClient schedulerClient = new SchedulerClientFactory(config,
runtime)
+ .getSchedulerClient();
+ if (!schedulerClient.killTopology(killTopologyRequest)) {
+ final String logMessage =
+ String.format("Failed to remove topology '%s' from scheduler
after failed submit. "
+ + "Please re-try the kill command.", topologyName);
+ errorMessage.append(String.format("%n%s", logMessage));
+ LOG.log(Level.SEVERE, logMessage);
+ }
+ // SUPPRESS CHECKSTYLE IllegalCatch
+ } catch (Exception ignored){
+ // The above call to clear the Scheduler may fail. This situation can
be ignored.
Review comment:
Could be useful to have the info in the errorMessage I feel.
##########
File path: deploy/kubernetes/general/apiserver.yaml
##########
@@ -95,6 +95,7 @@ spec:
-D
heron.uploader.dlog.topologies.namespace.uri=distributedlog://zookeeper:2181/heron
-D
heron.statefulstorage.classname=org.apache.heron.statefulstorage.dlog.DlogStorage
-D
heron.statefulstorage.dlog.namespace.uri=distributedlog://zookeeper:2181/heron
+ # -D heron.kubernetes.pod.template.configmap.disabled=true
Review comment:
do we want to remove this line or keep 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
