Skip to content

fix(deployer): read since as an absolute epoch on Kubernetes - #30

Merged
thxCode merged 1 commit into
gpustack:mainfrom
yxf0314:issue/6181-large-serving-logs
Sep 22, 2026
Merged

thxCode merged 1 commit into
gpustack:mainfrom
yxf0314:issue/6181-large-serving-logs

Conversation

@yxf0314

@yxf0314 yxf0314 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

The abstract deployer declares since as an absolute epoch in seconds, and Docker and Podman implement it that way, but the Kubernetes implementation hands the value straight to sinceSeconds, which the API reads as a duration counted back from now. An epoch arrives as roughly 54 years, so the filter silently matches everything: a caller resuming a log stream from a cursor gets the whole log back instead, with no error to notice it by. Reproduced on minikube v1.34.0 — since_seconds=1789884554 returned all 29 lines, byte for byte what not filtering returns.

to_since_seconds converts inside the deployer, because the installed Python client exposes only sinceSeconds and has no since_time to pass an instant through. It rounds up and adds a second so the requested instant lands inside the window rather than just outside it, and the caller sees a couple of seconds of older lines rather than a gap — measured at two extra lines, never a missing one. It also floors the result at 1: a cursor ahead of the node's clock would otherwise produce a zero or negative duration, which the API ignores, which is the same "returns everything" failure by another route.

The facade docstrings said "since a given time (in seconds)", which reads either way and is how the two implementations came to disagree in the first place. They now say the epoch outright, and the Kubernetes parameter doc that claimed a relative duration follows the facade rather than contradicting it.

The same log paths also fail on their own declared default. tail defaults to None, and both Kubernetes paths plus the Docker and Podman endoscopic ones compare it against zero before the API sees it, so a caller relying on the default gets a TypeError rather than every line. Docker's and Podman's workload paths pass tail through untouched and were never affected, which is most of why this has gone unnoticed; on Kubernetes the reachable caller is a benchmark log snapshot, where the exception is swallowed into a log line and shows up only as a snapshot that came back empty.

Verified with 20 cases in tests/gpustack_runtime/deployer/test_logs_options.py: Docker and Podman forward the epoch untouched, both Kubernetes paths convert it, since=None stays unset on all four, the declared tail default reaches the API, and to_since_seconds never returns a non-positive duration. Full suite 671 passed, 20 skipped.

This unblocks a timestamp-cursor log resume on the GPUStack side, which needs since to mean the same thing on every deployer. ref gpustack/gpustack#6181

The abstract deployer declares `since` as an absolute epoch in seconds, and
Docker and Podman implement it that way, but the Kubernetes implementation
hands the value straight to sinceSeconds, which the API reads as a duration
counted back from now. An epoch arrives as roughly 54 years, so the filter
silently matches everything: a caller resuming a log stream from a cursor
gets the whole log back instead, with no error to notice it by. Convert
inside the deployer, rounding up and adding a second so the requested instant
lands inside the window rather than just outside it, and keep the result
positive -- a cursor ahead of the node's clock would otherwise produce a
duration the API ignores.

The same log paths also fail on their own declared default. `tail` defaults
to None, and the two Kubernetes paths and both endoscopic ones compare it
against zero before the API sees it, so a caller relying on the default gets
a TypeError rather than every line. Fetching a workload's logs without a tail
is exactly how a benchmark snapshot reads them.

Task 2 of large-serving-logs.

Signed-off-by: yxf <xf.ye@gpustack.ai>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes and corrects how log retrieval options, specifically since (as an absolute epoch in seconds) and tail, are handled across the Docker, Podman, and Kubernetes deployers. It introduces a to_since_seconds helper in the Kubernetes deployer to properly convert absolute epochs into positive relative durations as required by the Kubernetes API, and ensures that tail handles None values gracefully without causing comparison errors. Additionally, a comprehensive test suite has been added to verify these behaviors. There are no review comments to address, and the changes look solid.

@yxf0314
yxf0314 requested a review from thxCode September 22, 2026 03:41

@thxCode thxCode 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.

LGTM

@thxCode
thxCode merged commit 327b2fb into gpustack:main Sep 22, 2026
7 checks passed
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.

2 participants