Skip to content

[#10865] improvement(helm): Add priorityClassName support to Gravitino Helm charts#11698

Open
dennismdejong wants to merge 2 commits into
apache:mainfrom
dennismdejong:fix/helm-priority-class-name
Open

[#10865] improvement(helm): Add priorityClassName support to Gravitino Helm charts#11698
dennismdejong wants to merge 2 commits into
apache:mainfrom
dennismdejong:fix/helm-priority-class-name

Conversation

@dennismdejong

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add priorityClassName support to all three Gravitino Helm charts (gravitino, gravitino-iceberg-rest-server, gravitino-lance-rest-server).

Why are the changes needed?

The Helm charts do not expose priorityClassName on the pod spec. On shared clusters where batch jobs compete with production services, Kubernetes uses PriorityClass to decide which pods to evict. Without this, Gravitino pods have no priority tier and can be evicted by lower-priority workloads during resource pressure, causing unexpected downtime for production catalog services.

Fix: #10865

Does this PR introduce any user-facing change?

Yes — adds a new priorityClassName value to all three charts:

  • dev/charts/gravitino/values.yaml
  • dev/charts/gravitino-iceberg-rest-server/values.yaml
  • dev/charts/gravitino-lance-rest-server/values.yaml

Default is empty string (""), which omits the field — fully backward compatible.

How was this patch tested?

  • helm lint on all 3 charts — passed
  • helm template — verified priorityClassName renders when set, omitted when empty
  • helm unittest — added test cases for all 3 charts (set + omitted scenarios):
gravitino:      33 passed, 33 total
iceberg-rest:   24 passed, 24 total
lance-rest:     24 passed, 24 total

@danhuawang danhuawang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! The priorityClassName feature looks good overall. A couple of comments:

1. Please split this PR — it contains two unrelated changes.

Commit 97566fd (making the init-postgresql secret name configurable, #11696) is unrelated to the priorityClassName feature (#10865). Please remove it from this PR and submit it separately. Mixing unrelated changes makes review and bisect harder.

2. (Nit) Missing blank line in lance-rest-server deployment.yaml

The priorityClassName block is added directly after the tolerations {{- end }} without a separating blank line, inconsistent with the other two charts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Add priorityClassName support to Gravitino Helm charts

2 participants