Copilot commented on code in PR #6723:
URL: https://github.com/apache/hive/pull/6723#discussion_r3863083029
##########
packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/autoscaling/ComponentAutoscaler.java:
##########
@@ -94,8 +94,13 @@ public EvaluationResult evaluate(List<PodMetrics> metrics,
AutoscalingSpec spec,
int target;
if (clamped > currentReplicas) {
- // Scale up: use stabilized max (highest recommendation in window —
don't under-scale)
- target = scaleUpWindow.stabilizedMax();
+ if (component.startsWith(ConfigUtils.COMPONENT_LLAP + "-")) {
+ // HS2 sessions activation gate scales up the LLAP pods to atleast 1
+ // in presence of sessions. Avoid stabilizedMin in this start-up case.
+ target = currentReplicas == 0 ? clamped :
scaleUpWindow.stabilizedMin();
Review Comment:
In the scale-up path for LLAP components, this uses
scaleUpWindow.stabilizedMin(), which returns the *lowest* recommendation in the
stabilization window. That can under-scale on scale-up and contradicts both the
surrounding comment and StabilizationWindow’s documented semantics (max for
scale-up, min for scale-down).
##########
packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/autoscaling/LlapScalingStrategy.java:
##########
@@ -28,11 +28,11 @@
/**
* Scaling strategy for LLAP daemons.
- * Formula: avg(QueuedRequests + Configured - Available) across all pods.
- * This represents average "busy slots + queued" per daemon.
- * desired = ceil(avg_busy / scaleUpThreshold)
+ * For Scale-Up: Pending Load across all TezAM pods should be above the
threshold.
+ * For Scale-Down: Running Load across all LLAP pods should be below the
threshold.
+ * desired = ceil(totalClusterLoad / capacityPerDaemon)
* <p>
- * Activation gate: only scale if HS2 has open sessions (prevents zombie
scaling).
+ * Activation gate: only scale if HS2 has open sessions & TezAMs are running
DAGs (prevents zombie scaling).
Review Comment:
The class-level Javadoc says the activation gate requires "TezAMs are
running DAGs", but the implementation never checks the tez_am_dag_running
metric (only HS2 sessions and whether TezAM metrics are present / load is
non-zero). This makes the comment misleading for future maintainers.
##########
packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/autoscaling/LlapScalingStrategy.java:
##########
@@ -83,36 +85,57 @@ public int computeDesiredReplicas(List<PodMetrics>
podMetrics,
return minReplica;
}
- // Compute average busy slots across all LLAP pods
- double totalBusy = 0;
- int podCount = 0;
+ List<PodMetrics> tezAmMetrics =
orchestrator.getTezAmMetricsFromCache(cluster, llapName);
+ double totalPending = 0;
+ for (PodMetrics pm : tezAmMetrics) {
+ totalPending += pm.metrics().getOrDefault(METRIC_TEZ_PENDING_TASKS, 0.0);
+ }
+
+ double totalLLAPCapacity = 0;
+ double totalLLAPLoad = 0;
for (PodMetrics pm : podMetrics) {
double queued = pm.metrics().getOrDefault(METRIC_QUEUED, 0.0);
double configured = pm.metrics().getOrDefault(METRIC_CONFIGURED, 0.0);
double available = pm.metrics().getOrDefault(METRIC_AVAILABLE, 0.0);
- double busy = queued + configured - available;
- totalBusy += busy;
- podCount++;
+ totalLLAPCapacity +=
pm.metrics().getOrDefault(METRIC_MAX_FREE_SLOTS_CONFIGURED, 0.0);
+ totalLLAPLoad += queued + configured - available;
Review Comment:
totalLLAPCapacity is derived solely from METRIC_MAX_FREE_SLOTS_CONFIGURED.
If that metric is absent (e.g., older LLAP image / exporter rule mismatch),
getOrDefault(...) yields 0.0 and capacityPerDaemon falls back to 1.0 later,
which can produce wildly incorrect desired replicas. Consider falling back to
the existing configured-executors metric when this metric is missing, and clamp
load to non-negative.
--
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]
