server-stream: follow-up on SSE Replay Buffer (#23226)#25047
Open
ServeurpersoCom wants to merge 6 commits into
Open
server-stream: follow-up on SSE Replay Buffer (#23226)#25047ServeurpersoCom wants to merge 6 commits into
ServeurpersoCom wants to merge 6 commits into
Conversation
address review from ggerganov: scope the public stream functions under the server_stream_ prefix, matching server_stream_session_manager_start/stop.
address review from ggerganov: make done, completed_ts and the GC running flag plain members under their mutex and set the condvar predicates under the lock. keep cancelled atomic for the lock-free should_stop poll.
address review from ggerganov: drop comments that restate the code, keep the concurrency, lifetime and ordering rationale. de-stale a few comments left by the pimpl: g_stream_sessions is now internal and the /v1/streams listing is gone.
reflect server_stream_session_manager_start/stop and the server_stream_ prefix, note the manager is now a file-static singleton hidden in the .cpp
keep the bring-up traces for diagnostics but off the default log: skip drain, draining, drain ended, DELETE evict, attach_pipe, and the router stream resume proxy.
github-actions
Bot
added
documentation
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
2 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.
Overview
Follow-up to #23226 addressing @ggerganov review
No functional change to the resumable stream behavior.
Additional information
pimpl: the stream_session_manager implementation is hidden in the .cpp, built on top of d5b1b88. the header only exposes the pipe surface and the route factories.
the manager is a file-static singleton driven by server_stream_session_manager_start/stop, no extern.
public stream functions scoped under the server_stream_ prefix.
done, completed_ts and the GC running flag are plain members under their mutex, with the condvar predicates set under the lock. cancelled stays atomic for the lock-free should_stop poll.
comments trimmed to the concurrency, lifetime and ordering rationale.
bring-up traces moved to SRV_DBG so they stay available for diagnostics without polluting the default log.
dev docs updated for the pimpl and the server_stream_ prefix.
Requirements