Repository navigation
fix(sglang): [cherry-pick 1.5.1] support min_tokens on tokenizer-free decode workers (#14276, #14378) #14951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8188274
d2642e5
d845244
5b8122b
62a3d12
1e57f80
672a608
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -360,12 +360,25 @@ def _build_sampling_params(self, request: Dict[str, Any]) -> Dict[str, Any]: | |||||||||
| _plain = stop_conditions.get("stop_token_ids") or [] | ||||||||||
| _merged = list(set(_hidden).union(_plain)) | ||||||||||
| stop_token_ids = _merged if _merged else None | ||||||||||
| # A tokenizer-free SGLang rejects positive min_new_tokens. Let | ||||||||||
| # Dynamo's decoder enforce the floor and keep the engine running so | ||||||||||
| # it cannot stop before Dynamo reaches it. | ||||||||||
| min_tokens = stop_conditions.get("min_tokens") | ||||||||||
| hold_engine_open = bool( | ||||||||||
| min_tokens | ||||||||||
| and min_tokens > 0 | ||||||||||
| and self.config.server_args.skip_tokenizer_init | ||||||||||
| ) | ||||||||||
| if hold_engine_open: | ||||||||||
| stop_token_ids = None | ||||||||||
|
|
||||||||||
| param_mapping = { | ||||||||||
| "n": sampling_opts.get("n"), | ||||||||||
| "max_new_tokens": stop_conditions.get("max_tokens"), | ||||||||||
| "min_new_tokens": stop_conditions.get("min_tokens"), | ||||||||||
| "ignore_eos": stop_conditions.get("ignore_eos"), | ||||||||||
| "min_new_tokens": None if hold_engine_open else min_tokens, | ||||||||||
| "ignore_eos": True | ||||||||||
| if hold_engine_open | ||||||||||
| else stop_conditions.get("ignore_eos"), | ||||||||||
| "stop_token_ids": stop_token_ids, | ||||||||||
| **_sampling_option_params(sampling_opts), | ||||||||||
| **self._get_guided_decoding_params( | ||||||||||
|
|
@@ -691,16 +704,23 @@ async def _process_token_stream( | |||||||||
| """ | ||||||||||
| # Use Future pattern for request ID - will be set when first response arrives | ||||||||||
| request_id_future: asyncio.Future[str] = asyncio.Future() | ||||||||||
| request_ids: set[str] = set() | ||||||||||
| first_output_seen = False | ||||||||||
| async with self._cancellation_monitor(request_id_future, context): | ||||||||||
| async with self._cancellation_monitor(request_id_future, context, request_ids): | ||||||||||
| async for res in stream_source: | ||||||||||
| meta_info = res.get("meta_info", {}) | ||||||||||
| # Extract SGLang request ID from the first response and set the future | ||||||||||
| if not request_id_future.done(): | ||||||||||
| sglang_request_id = meta_info.get("id") | ||||||||||
| if sglang_request_id: | ||||||||||
| sglang_request_id = meta_info.get("id") | ||||||||||
| if sglang_request_id: | ||||||||||
| request_ids.add(sglang_request_id) | ||||||||||
| if not request_id_future.done(): | ||||||||||
| request_id_future.set_result(sglang_request_id) | ||||||||||
| logging.debug(f"New SGLang Request ID: {sglang_request_id}") | ||||||||||
| if meta_info.get("finish_reason"): | ||||||||||
| request_ids.discard(sglang_request_id) | ||||||||||
|
Comment on lines
+718
to
+719
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The premise does not hold on the engine this branch pins, so I do not think there is anything to fix here. SGLang 0.5.18 gives each choice its own id under n > 1
Each choice therefore carries a distinct What I did not verify: a live multi-choice generation on a GPU. This is read from the pinned image source plus the parameter path, not from a served request.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-checked this round, and the premise still does not hold. The engine pin has not moved, so each choice still gets its own identifier and there is nothing to fix here. The pin, read at three refs
The release branch changed |
||||||||||
| if context.is_stopped(): | ||||||||||
| # A choice's first chunk can arrive after the monitor fired. | ||||||||||
| self._abort_requests(request_ids, context) | ||||||||||
| continue | ||||||||||
|
|
||||||||||
| # Check cancellation before yielding to allow proper cleanup. | ||||||||||
| # This lets SGLang proceed to the second token generation, which will | ||||||||||
|
|
@@ -829,16 +849,22 @@ async def _process_text_stream( | |||||||||
|
|
||||||||||
| # Use Future pattern for request ID - will be set when first response arrives | ||||||||||
| request_id_future: asyncio.Future[str] = asyncio.Future() | ||||||||||
| request_ids: set[str] = set() | ||||||||||
| first_output_seen = False | ||||||||||
| async with self._cancellation_monitor(request_id_future, context): | ||||||||||
| async with self._cancellation_monitor(request_id_future, context, request_ids): | ||||||||||
| async for res in stream_source: | ||||||||||
| meta_info = res.get("meta_info", {}) | ||||||||||
| # Extract SGLang request ID from the first response and set the future | ||||||||||
| if not request_id_future.done(): | ||||||||||
| sglang_request_id = meta_info.get("id") | ||||||||||
| if sglang_request_id: | ||||||||||
| sglang_request_id = meta_info.get("id") | ||||||||||
| if sglang_request_id: | ||||||||||
| request_ids.add(sglang_request_id) | ||||||||||
| if not request_id_future.done(): | ||||||||||
| request_id_future.set_result(sglang_request_id) | ||||||||||
| logging.debug(f"New SGLang Request ID: {sglang_request_id}") | ||||||||||
| if meta_info.get("finish_reason"): | ||||||||||
| request_ids.discard(sglang_request_id) | ||||||||||
| if context.is_stopped(): | ||||||||||
| self._abort_requests(request_ids, context) | ||||||||||
| continue | ||||||||||
|
|
||||||||||
| # Check cancellation before yielding to allow proper cleanup. | ||||||||||
| # This lets SGLang proceed to the second token generation, which will | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] The pick stops two commits short of #14276.
components/src/dynamo/sglang/request_handlers/llm/decode_handler.py:792got the per-choice abort set, but_process_text_streamat line 990 did not. A cancelledn>1text request still aborts one choice. Please also pick21638df03dandc66ce8a852.The two commits that landed on #14276 after this pick was cut
The pick commit
c14512f3was authored 2026-09-16T19:10Z. #14276 merged 2026-09-17T00:12Z. Three commits landed in between:7ca35cee32test(sglang): provide stop state in shutdown context stubc66ce8a852fix(sglang): retain context logging after request aborts21638df03dfix(sglang): abort every unfinished text-stream choice21638df03dis the functional one. It gives_process_text_streamthe same treatment_process_token_streamalready has here:Without it,
_cancellation_monitorfalls back to{sglang_request_id}, the id of the first chunk only. SGLang gives each choice its own id undern>1:tokenizer_manager.pycallstmp_obj.regenerate_rid()once perparallel_sample_numin the SGLang 0.5.18 image. So the other choices keep generating tomax_new_tokensafter the client goes away.c66ce8a852is logging only. It addscontextto_abort_requestsand onelogging.infoline. Left as it is,handler_base.pyanddecode_handler.pydiffer frommainin signature, which makes the next pick on these files conflict.Both are still absent at head
859c8f27. Checked withgh api repos/ai-dynamo/dynamo/commits/<sha>against the head tree.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] still open at head
859c8f27, and I was wrong to callc66ce8a852"logging only". The pick deleteslogging.info(f"Aborted Request ID: {context.id()}")fromcomponents/src/dynamo/sglang/request_handlers/handler_base.py:1269and never puts it back, so the string is absent from that file. Please pick both commits.The deleted log line traced across refs, the runtime probe, and the test that polls for it
I re-ran this against the commit that merged on
main(e5ef9d778b, a single-parent squash), not the branch head.Count of
Aborted Request IDundercomponents/:handler_base.py55589a5e72:1269logging.info(f"Aborted Request ID: {context.id()}")74a5cef4da:1269, same linec14512f325859c8f2732release/1.5.0tip32b8b2f8c6:1202, presentmainbefore #14276232f881e07:1235, presentmainafter #14276e5ef9d778b:1179logging.info("Aborted Request ID: %s", context.id())#14276 removed the old line and re-added it inside
_abort_requestsinc66ce8a852. The pick took the removal and not the re-add. The hunk inc14512f325:Runtime probe in the sglang runtime-test image, PR tree at
/tmp/work, module path printed to prove which tree loaded:tests/fault_tolerance/cancellation/test_sglang.py:290and:409poll forf"Aborted Request ID: {request_id}". Both are markedgpu_2andpre_merge, and the lanesglang-runtime / 2-GPU Test cuda13.0, amd64selects them withpre_merge and sglang and gpu_2. I collected that expression against this tree and got 5 tests, 2 of them from that file. Both failed on this head:21638df03dis still missing too._process_text_streamatdecode_handler.py:990still callsself._cancellation_monitor(request_id_future, context)with two arguments, while_process_token_streamat:792passesrequest_ids.Per-file comparison of the pick against the
mainsquash, added and removed lines checksummed on each side:mainmainhandler_base.py12323242)a223981d)51548e4e)51548e4e)decode_handler.pyb2312646)559bc111)d4e1fdb3)55e00c9d)test_sglang_decode_handler.py2df75e94)2df75e94)5a2608ff)5a2608ff)test_stop_behavior.rsf9624810)f9624810)7e098327)7e098327)The two test files are byte-identical to
main. The two source files are not. The empty-string control for this checksum isda39a3ee, and no cell above carries it.No unit test pins either gap. Mutating
_process_text_streamto apply21638df03dleft the suite at111 passed, and the control mutation that stripsrequest_idsfrom_process_token_streamalso left it at111 passed. Both mutations landed: baseline sha1ee20a96fb26, mutatedd504aafd4da4and416258669c93, restored1ee20a96fb26both times. The liveness control proves the suite can fail: disabling thehold_engine_opengate gave1 failed, 110 passedontest_build_sampling_params_holds_tokenizer_free_engine_open_for_min_tokens.Not verified: a live multi-choice cancellation on two GPUs. This box has one.