YongGang commented on code in PR #14620:
URL: https://github.com/apache/druid/pull/14620#discussion_r1270012678
##########
extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/common/KubernetesPeonClient.java:
##########
@@ -212,47 +212,46 @@ private List<Job> getJobsToCleanup(List<Job> candidates,
long howFarBack, TimeUn
return toDelete;
}
- public Optional<Pod> getPeonPod(K8sTaskId taskId)
+ public Optional<Pod> getPeonPod(String jobName)
{
- return clientApi.executeRequest(client -> getPeonPod(client, taskId));
+ return clientApi.executeRequest(client -> getPeonPod(client, jobName));
}
- private Optional<Pod> getPeonPod(KubernetesClient client, K8sTaskId taskId)
+ private Optional<Pod> getPeonPod(KubernetesClient client, String jobName)
{
List<Pod> pods = client.pods()
.inNamespace(namespace)
- .withLabel("job-name", taskId.getK8sTaskId())
+ .withLabel("job-name", jobName)
.list()
.getItems();
return pods.isEmpty() ? Optional.absent() : Optional.of(pods.get(0));
}
- public Pod getPeonPodWithRetries(K8sTaskId taskId)
+ public Pod getPeonPodWithRetries(String jobName)
{
- return clientApi.executeRequest(client -> getPeonPodWithRetries(client,
taskId, 5, RetryUtils.DEFAULT_MAX_TRIES));
+ return clientApi.executeRequest(client -> getPeonPodWithRetries(client,
jobName, 5, RetryUtils.DEFAULT_MAX_TRIES));
}
@VisibleForTesting
- Pod getPeonPodWithRetries(KubernetesClient client, K8sTaskId taskId, int
quietTries, int maxTries)
+ Pod getPeonPodWithRetries(KubernetesClient client, String jobName, int
quietTries, int maxTries)
{
- String k8sTaskId = taskId.getK8sTaskId();
try {
return RetryUtils.retry(
() -> {
- Optional<Pod> maybePod = getPeonPod(client, taskId);
+ Optional<Pod> maybePod = getPeonPod(client, jobName);
if (maybePod.isPresent()) {
return maybePod.get();
}
throw new KubernetesResourceNotFoundException(
"K8s pod with label: job-name="
- + k8sTaskId
+ + jobName
+ " not found");
},
DruidK8sConstants.IS_TRANSIENT, quietTries, maxTries
);
}
catch (Exception e) {
- throw new KubernetesResourceNotFoundException("K8s pod with label:
job-name=" + k8sTaskId + " not found");
+ throw new KubernetesResourceNotFoundException("K8s pod with label:
job-name=" + jobName + " not found");
Review Comment:
If we pass `jobName` instead of `K8sTaskId`, we lose the `originalTaskId` in
the log, not sure if it's useful for troubleshooting.
##########
extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/common/KubernetesOverlordUtils.java:
##########
@@ -43,4 +45,10 @@ public static String convertTaskIdToK8sLabel(String taskId)
return taskId == null ? "" :
StringUtils.left(RegExUtils.replaceAll(taskId, K8S_TASK_ID_PATTERN, "")
.toLowerCase(Locale.ENGLISH), 63);
}
+
+ public static String convertTaskIdToJobName(String taskId)
Review Comment:
Do we still need to do this conversion if taskId is less than 63 chars? I
mean whether it's valuable to keep taskId same as JobName if it's possible.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
