HIVE-30033: Hive K8s operator: log replica changes - #6773
Open
abstractdog wants to merge 2 commits into
Open
abstractdog wants to merge 2 commits into
abstractdog wants to merge 2 commits into
Conversation
abstractdog
force-pushed
the
HIVE-30033
branch
from
September 10, 2026 09:42
5959b04 to
28daa2a
Compare
asf-ci-hive
added
tests failed
tests pending
tests passed
and removed
tests pending
tests failed
labels
abstractdog
force-pushed
the
HIVE-30033
branch
from
September 16, 2026 10:14
28daa2a to
5e88135
Compare
|
deniskuzZ
reviewed
Sep 17, 2026
| } | ||
| Integer current = Workloads.replicas(existing.get()); | ||
| if (current != null && !current.equals(desired)) { | ||
| LOG.info("Scaling {} {}/{}: {} -> {} replicas", component, ns, name, current, desired); |
Member
There was a problem hiding this comment.
Maybe "Setting replica count for {} to {}?" maybe we can join both crate and scale ?
deniskuzZ
reviewed
Sep 17, 2026
| Optional<R> existing = getSecondaryResource(primary, context); | ||
| Integer resolved = computeReplicaCount(primary, existing, autoscaling, | ||
| staticReplicas, initialReplicas); | ||
| logReplicaChange(primary, existing, resolved); |
Member
There was a problem hiding this comment.
do we log even when existing is empty? log on boostrap, first creation ?
deniskuzZ
reviewed
Sep 17, 2026
| ? s.getSpec().getReplicas() : initialReplicas; | ||
| } | ||
| return initialReplicas; | ||
| Integer current = Workloads.replicas(existing.get()); |
Member
There was a problem hiding this comment.
can the current be null if existing.isPresent()?
deniskuzZ
reviewed
Sep 17, 2026
| */ | ||
| private void logReplicaChange(P primary, Optional<R> existing, Integer desired) { | ||
| String component = getComponentName(); | ||
| if (component == null || desired == null) { |
deniskuzZ
reviewed
Sep 17, 2026
| @@ -208,17 +208,22 @@ public static PodDisruptionBudget buildPdb(HiveCluster hc, LlapSpec llap) { | |||
|
|
|||
| /** TezAM Deployment/Service name for a specific LLAP cluster. */ | |||
Member
There was a problem hiding this comment.
For TezAM we have statefullset, not deployment, no? That actually makes me wonder why we have deployment for LLAP that doesn't allow parallel botstrap and rolling upgrade is sequential
deniskuzZ
reviewed
Sep 17, 2026
| * operator was doing nothing. Read failures are swallowed at DEBUG: the SSA below runs either | ||
| * way, and a missing pre-scale line is not worth failing the reconcile over. | ||
| */ | ||
| private void logReplicaChange(KubernetesClient client, String ns, String workloadName, |
Member
There was a problem hiding this comment.
can we generalize log method or need both?
deniskuzZ
reviewed
Sep 17, 2026
| * Returns spec.replicas from a Deployment or StatefulSet, or null when the resource is | ||
| * absent, has no spec, or the field is unset. A non-workload resource returns null too. | ||
| */ | ||
| public static Integer replicas(HasMetadata resource) { |
Member
There was a problem hiding this comment.
iDK, should it be optional? replicas can't be null unless deployment/sts not running
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Log an INFO line in the reconciler whenever a HiveCluster component is created or scaled to a different replica count. Covers LLAP, TezAM, HiveServer2 and Metastore.
Why are the changes needed?
Scales driven by an SSA on the CR (helm upgrade,
kubectl edit, a tool changing.spec.*.replicas) reached the workload silently — only the autoscaler path logged. On a scale that didn't converge you couldn't tell from the operator log whether the reconciler had seen the change at all.Does this PR introduce any user-facing change?
Operator log only. Example:
INFO HiveDependentResource: Scaling hiveserver2 lbodor-perf/hive-hiveserver2: 2 -> 1 replicas
INFO HiveDependentResource: Scaling metastore lbodor-perf/hive-metastore: 2 -> 1 replicas
INFO HiveClusterReconciler: Scaling llap lbodor-perf/hive-llap0: 12 -> 15 replicas
Silence still means "no change" — the log fires only on a real delta.
How was this patch tested?
Manually on an EKS cluster: scaled HS2, Metastore and LLAP up and down; confirmed the "Scaling …: A -> B replicas" line on each real change and no line when the count already matched.