surahman commented on a change in pull request #3710:
URL: https://github.com/apache/incubator-heron/pull/3710#discussion_r728231089
##########
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:
Hi @nwangtw, thank you for the review 😄.
This is a pertinent question and an equally good point. I left it there so
that people are aware that the feature can be disabled and to make it easy to
switch it off without having to look anything up. I am indifferent about
discarding it but feel it should be there.
##########
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:
Good catch, I shall affect this change.
##########
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:
You are right, I will make the update.
##########
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:
I was reluctant to add a `log` message because of the rather dense
logging on the API server. If an error occurs from this shutdown attempt it
will be because the Scheduler was unreachable. What I can do is log a message
at the `FINER` or `INFO` levels.
--
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]
