kubeflow / kubeflow/sdk

_memory_kubernetes_to_spark does not preserve byte count when converting decimal Kubernetes memory units (K/M/G/T)

Open Beginner friendly
#581 4 comments 0 reactions 0 assignees View on GitHub
area/spark kind/bug
Dominant language
Python
Stars
148
Forks
262
Avg merge
1d 2h
Merged PRs (30d)
1

Description

## What happened?

`_memory_kubernetes_to_spark()` converts Kubernetes-style memory quantities into Spark/JVM-style memory strings before submitting a Spark Connect session.

Kubernetes resource quantities support two distinct families of memory units:

| Unit Family | Examples | Base |
|-------------|----------|------|
| Binary | `Ki`, `Mi`, `Gi`, `Ti` | Powers of 1024 |
| Decimal | `K`, `M`, `G`, `T` | Powers of 1000 |

The current implementation correctly maps binary suffixes (e.g. `Mi → m`, `Gi → g`).

However, decimal suffixes are effectively passed through after lowercasing (e.g. `M → m`, `G → g`) without converting the numeric value.

Spark/JVM memory suffixes (`k`, `m`, `g`, `t`) are always interpreted as **binary (1024-based)** units, regardless of case.

As a result, decimal Kubernetes quantities are silently interpreted as binary quantities, increasing the requested byte count.

### Example

| Input | Intended Bytes | Current Output | Spark Interprets As | Difference |
|-------|---------------:|---------------|--------------------:|-----------:|
| `512M` | 512,000,000 | `512m` | 536,870,912 bytes | +4.86% |
| `4G` | 4,000,000,000 | `4g` | 4,294,967,296 bytes | +7.37% |
| `1T` | 1,000,000,000,000 | `1t` | 1,099,511,627,776 bytes | +9.95% |

Depending on how a pod's memory limit is configured, this can cause the Spark driver or executor JVM to request more memory than the Kubernetes container is actually allowed, increasing the risk of an `OOMKilled` termination rather than a clean failure.

---

## Verification

This behavior is confirmed against Spark's implementation and the dependency versions currently used by this SDK.

- Spark's `JavaUtils.byteStringAsBytes` (used for parsing memory configuration values) treats **unsuffixed numeric values as bytes**.
- `Utils.memoryStringToMb` in Spark core explicitly converts values through bytes first because **a value without a suffix is assumed to already represent bytes**.
- Therefore, returning a plain integer (e.g. `512000000`) is an unambiguous and version-safe way to preserve the exact byte count.

Additionally:

- Spark only recently gained support for explicit Kubernetes-style suffixes (`Ki`, `Mi`, `Gi`, etc.) through **SPARK-55051** (merged January 2026).
- This repository currently targets **Spark 4.0.1**:
- `pyspark-connect==4.0.1` (pinned in `pyproject.toml`)
- `constants.DEFAULT_SPARK_VERSION = "4.0.1"`

Since Spark 4.0.1 predates SPARK-55051, relying on `Ki`/`Mi`/`Gi` suffixes would not be safe for the version currently supported by this SDK.

Returning the exact byte count as a plain integer avoids this compatibility issue.

---

## Steps to Reproduce

```python
from kubeflow.spark.backends.kubernetes.utils import _memory_kubernetes_to_spark

print(_memory_kubernetes_to_spark("512M"))
print(_memory_kubernetes_to_spark("4G"))
```

### Current Output

```text
512m
4g
```

---

## Expected Behavior

The conversion should preserve the original byte count exactly.

For example:

- `512M` (512,000,000 bytes)

should become

```text
512000000
```

rather than

```text
512m
```

because Spark interprets an unsuffixed integer as an exact byte count.

---
## Actual Behavior

Decimal Kubernetes units (`K`, `M`, `G`, `T`) are converted only by changing the suffix to lowercase without converting the numeric value.

Consequently, Spark interprets them as binary units, resulting in a larger memory allocation than originally requested.

---

## Suggested Fix

Handle binary and decimal Kubernetes units separately.

1. Parse the Kubernetes quantity and its suffix.
2. If the suffix is binary (`Ki`, `Mi`, `Gi`, `Ti`), keep the existing direct suffix mapping since these represent the same number of bytes.
3. If the suffix is decimal (`K`, `M`, `G`, `T`):
- Compute the exact byte count (`value × 1000ⁿ`).
- Return the result as a plain integer string (no suffix), which Spark interprets as bytes.
4. Leave already-Spark-format lowercase values (e.g. `512m`) unchanged for backward compatibility.

This preserves the requested memory exactly for both Kubernetes unit families while remaining compatible with the Spark version currently targeted by the SDK.

---

### Environment

- **Repository:** `kubeflow/sdk`
- **Branch:** `main`
- **Component:** `kubeflow.spark` (`SparkClient`)
- **Target Spark Version:** `4.0.1`

Can any maintainer please verify my approach so that I can implement the fix and generate a PR ?

### Impacted by this bug?

Give it a 👍 We prioritize the issues with most 👍

Contributor guide

Open the contributing guide

Research direction

Start with kubeflow/spark/backends/kubernetes/utils.py and inspect _memory_kubernetes_to_spark(), then check pyproject.toml and constants.DEFAULT_SPARK_VERSION for the supported Spark version. Verify decimal K/M/G/T inputs preserve their exact byte count while binary suffixes and existing lowercase Spark-format values retain their current behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
kubernetes, python, spark
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.