From 24c42c90be82c24433ab10e65db24e52a48d8084 Mon Sep 17 00:00:00 2001 From: Lianmin Zheng Date: Sun, 5 Jul 2026 23:05:07 -0700 Subject: [PATCH] Clean up ServerArgs post-init dispatch (#30186) --- python/sglang/srt/server_args.py | 118 +++++++++------- .../unit/managers/test_mm_process_config.py | 24 ++-- .../unit/server_args/test_server_args.py | 127 +++++++++--------- 3 files changed, 147 insertions(+), 122 deletions(-) diff --git a/python/sglang/srt/server_args.py b/python/sglang/srt/server_args.py index 5492cee7a..1f403309b 100644 --- a/python/sglang/srt/server_args.py +++ b/python/sglang/srt/server_args.py @@ -2614,6 +2614,25 @@ class ServerArgs: def __post_init__(self): """ Orchestrates the handling of various server arguments, ensuring proper configuration and validation. + + Dispatcher style principles: + 1. Keep this method as an ordered dispatcher. Each step should be a + named self._handle_* call; put imports, conditionals, mutations, and + raises inside helpers instead of inline here. + 2. Keep the dummy-model boundary as early as correctness allows. Only + model-independent bootstrap, API/network/protocol validation, and + errors that should fire for dummy models should run before it. + 3. Order handlers by dependency domains, not by historical insertion: + internal/bootstrap, API/network/protocol, model source/path + resolution, hardware/platform, model-specific adjustment, + parallelism, kernel/attention backend, cuda graph, memory/cache, + and advanced/debug features. + 4. Hide narrow integrations behind general handler names. The + dispatcher should say what phase is being handled, not expose a + vendor-, hook-, or feature-specific implementation detail. + 5. Give each handler one clear contract: what state it expects, what it + may mutate, and whether it validates only. Long ordering comments + belong in the helper or signal that the helper should be split. """ # Declaration stash for the override/post-process passes. Set before any @@ -2622,58 +2641,36 @@ class ServerArgs: # _handle_model_specific_adjustments never runs. self._resolved_overrides = [] - self._maybe_download_model_for_runai() - - # Normalize load balancing defaults early (before dummy-model short-circuit). - self._handle_load_balance_method() - - # Validate mm_process_config before dummy-model early return. - self._handle_multimodal() - # Validate SSL arguments early (before dummy-model short-circuit). - self._handle_ssl_validation() - # Validate transcription/ASR-specific server args (model-independent). - self._handle_asr_validation() - - # Validate PD disaggregation flags early (before dummy-model short-circuit). - from sglang.srt.arg_groups.pd_disaggregation_hook import ( - handle_pd_disaggregation, - ) - - handle_pd_disaggregation(self) - if self.enable_session_radix_cache and self.radix_eviction_policy != "priority": - raise ValueError( - "--enable-session-radix-cache requires --radix-eviction-policy priority" - ) - - # Normalize deprecated CP aliases before validations or model-specific - # defaults inspect enable_prefill_cp/cp_strategy. - self._handle_legacy_cp_arguments() - self._validate_prefill_only_disable_kv_cache_args() - self._handle_dcp_validation() - if self.model_path.lower() in ["none", "dummy"]: - # Skip for dummy models return + self._handle_model_source_paths() + + # Validate mm_process_config. + self._handle_multimodal() + # Validate SSL arguments early. + self._handle_ssl_validation() + # Validate transcription/ASR-specific server args. + self._handle_asr_validation() + # Handle deprecated arguments. self._handle_deprecated_args() # Handle deprecated environment variables for prefill delayer. self._handle_prefill_delayer_env_compat() - # Resolve --quantization unquant: explicitly opt out of quantization. - # Convert to None now (before model config validation), but record - # the intent so auto-detection in _handle_model_specific_adjustments - # does not override it. - if self.quantization == "unquant": - self.quantization = None - self._quantization_explicitly_unset = True - else: - self._quantization_explicitly_unset = False - # Set missing default values. self._handle_missing_default_values() + # Validate PD disaggregation flags before CUDA graph config. + self._handle_pd_disaggregation() + + # Normalize deprecated CP aliases before validations or model-specific + # defaults inspect enable_prefill_cp/cp_strategy. + self._handle_legacy_cp_arguments() + self._validate_prefill_only_disable_kv_cache_args() + self._handle_dcp_validation() + self._handle_cuda_graph_config() # Handle device-specific backends. @@ -2692,12 +2689,6 @@ class ServerArgs: # Handle memory-related, chunked prefill, and CUDA graph batch size configurations. self._handle_gpu_memory_settings(gpu_mem) - # enforce_disable_flashinfer_allreduce_fusion must be set before - # _handle_model_specific_adjustments, which auto-enables the fusion - # for several SM90/SM100 MoE arches. - if self.enable_deterministic_inference: - self.enforce_disable_flashinfer_allreduce_fusion = True - # Apply model-specific adjustments. self._handle_model_specific_adjustments() @@ -2736,6 +2727,9 @@ class ServerArgs: # Handle data parallelism. self._handle_data_parallelism() + # Normalize load balancing defaults. + self._handle_load_balance_method() + # Re-apply after model-specific defaults resolve attention_backend so # canonical CP mirrors to the right legacy runtime aliases. self._handle_legacy_cp_arguments() @@ -2792,7 +2786,8 @@ class ServerArgs: # Handle any other necessary validations. self._handle_other_validations() - def _maybe_download_model_for_runai(self): + def _handle_model_source_paths(self): + """Resolve model/tokenizer paths backed by remote object stores.""" if is_runai_obj_uri(self.model_path): ObjectStorageModel.download_and_get_path(self.model_path) @@ -2803,6 +2798,13 @@ class ServerArgs: ): ObjectStorageModel.download_and_get_path(self.tokenizer_path) + def _handle_pd_disaggregation(self): + from sglang.srt.arg_groups.pd_disaggregation_hook import ( + handle_pd_disaggregation, + ) + + handle_pd_disaggregation(self) + def _handle_dcp_validation(self): # Decode context parallel (DCP) is currently implemented and validated # only on AMD HIP/ROCm. Reject invalid or unverified configurations @@ -3006,7 +3008,16 @@ class ServerArgs: # - Otherwise, the draft model defaults to the same quantization as the target model. if self.speculative_draft_model_quantization is None: self.speculative_draft_model_quantization = self.quantization - elif self.speculative_draft_model_quantization == "unquant": + + # Resolve --quantization unquant before model config validation. Record + # the explicit opt-out so later auto-detection does not re-enable + # quantization. + if self.quantization == "unquant": + self.quantization = None + self._quantization_explicitly_unset = True + else: + self._quantization_explicitly_unset = False + if self.speculative_draft_model_quantization == "unquant": self.speculative_draft_model_quantization = None def _handle_modelscope_paths(self): @@ -3729,6 +3740,9 @@ class ServerArgs: is_deepseek_dsa, ) + if self.enable_deterministic_inference: + self.enforce_disable_flashinfer_allreduce_fusion = True + self.uses_mamba_radix_cache = False if parse_connector_type(self.model_path) == ConnectorType.INSTANCE: self._resolved_overrides = [] @@ -5177,8 +5191,7 @@ class ServerArgs: def _validate_prefill_only_disable_kv_cache_args(self): """Validate --prefill-only-disable-kv-cache flag/precondition constraints. - Runs before the dummy-model short-circuit so misuse is rejected even - for dummy models. Backend resolution is checked separately by + Backend resolution is checked separately by _handle_prefill_only_disable_kv_cache after backends settle. """ if not self.prefill_only_disable_kv_cache: @@ -5695,6 +5708,11 @@ class ServerArgs: envs.SGLANG_OPT_FP8_WO_A_GEMM.set(False) def _handle_cache_compatibility(self): + if self.enable_session_radix_cache and self.radix_eviction_policy != "priority": + raise ValueError( + "--enable-session-radix-cache requires --radix-eviction-policy priority" + ) + if self.enable_hierarchical_cache and self.disable_radix_cache: raise ValueError( "The arguments enable-hierarchical-cache and disable-radix-cache are mutually exclusive " diff --git a/test/registered/unit/managers/test_mm_process_config.py b/test/registered/unit/managers/test_mm_process_config.py index 202cbd4f7..6d5198687 100644 --- a/test/registered/unit/managers/test_mm_process_config.py +++ b/test/registered/unit/managers/test_mm_process_config.py @@ -11,41 +11,43 @@ register_amd_ci(est_time=1, suite="stage-b-test-1-gpu-small-amd") class TestMmProcessConfigValidation(unittest.TestCase): """Server-args validation for mm_process_config.""" + def _validate_config(self, mm_process_config): + args = ServerArgs(model_path="dummy", mm_process_config=mm_process_config) + args._handle_multimodal() + return args + def test_valid_config_accepted(self): - args = ServerArgs( - model_path="dummy", - mm_process_config={"image": {"max_pixels": 5000000}}, - ) + args = self._validate_config({"image": {"max_pixels": 5000000}}) self.assertEqual(args.mm_process_config, {"image": {"max_pixels": 5000000}}) def test_empty_config_accepted(self): - args = ServerArgs(model_path="dummy", mm_process_config={}) + args = self._validate_config({}) self.assertEqual(args.mm_process_config, {}) def test_none_config_defaults_to_empty_dict(self): - args = ServerArgs(model_path="dummy", mm_process_config=None) + args = self._validate_config(None) # None is kept as-is for dummy models (default happens after early return) # but for real models it would be set to {} self.assertIsNone(args.mm_process_config) def test_top_level_non_dict_rejected(self): with self.assertRaises(TypeError) as ctx: - ServerArgs(model_path="dummy", mm_process_config="bad") + self._validate_config("bad") self.assertIn("mm_process_config must be a dict", str(ctx.exception)) def test_modality_non_dict_rejected_image(self): with self.assertRaises(TypeError) as ctx: - ServerArgs(model_path="dummy", mm_process_config={"image": "bad"}) + self._validate_config({"image": "bad"}) self.assertIn("mm_process_config['image'] must be a dict", str(ctx.exception)) def test_modality_non_dict_rejected_video(self): with self.assertRaises(TypeError) as ctx: - ServerArgs(model_path="dummy", mm_process_config={"video": 123}) + self._validate_config({"video": 123}) self.assertIn("mm_process_config['video'] must be a dict", str(ctx.exception)) def test_modality_non_dict_rejected_audio(self): with self.assertRaises(TypeError) as ctx: - ServerArgs(model_path="dummy", mm_process_config={"audio": [1, 2]}) + self._validate_config({"audio": [1, 2]}) self.assertIn("mm_process_config['audio'] must be a dict", str(ctx.exception)) def test_multi_modality_config_accepted(self): @@ -54,7 +56,7 @@ class TestMmProcessConfigValidation(unittest.TestCase): "video": {"max_pixels": 602112}, "audio": {"sample_rate": 16000}, } - args = ServerArgs(model_path="dummy", mm_process_config=config) + args = self._validate_config(config) self.assertEqual(args.mm_process_config, config) diff --git a/test/registered/unit/server_args/test_server_args.py b/test/registered/unit/server_args/test_server_args.py index ed8ac52b4..5e63d9895 100644 --- a/test/registered/unit/server_args/test_server_args.py +++ b/test/registered/unit/server_args/test_server_args.py @@ -108,27 +108,34 @@ class TestMambaCacheStochasticRounding(unittest.TestCase): class TestLoadBalanceMethod(unittest.TestCase): + def _load_balance_args(self, **kwargs): + server_args = ServerArgs(model_path="dummy", **kwargs) + server_args._handle_pd_disaggregation() + server_args._handle_load_balance_method() + return server_args + def test_non_pd_defaults_to_round_robin(self): - server_args = ServerArgs(model_path="dummy", disaggregation_mode="null") + server_args = self._load_balance_args(disaggregation_mode="null") self.assertEqual(server_args.load_balance_method, "round_robin") def test_pd_prefill_defaults_to_follow_bootstrap_room(self): - server_args = ServerArgs(model_path="dummy", disaggregation_mode="prefill") + server_args = self._load_balance_args(disaggregation_mode="prefill") self.assertEqual(server_args.load_balance_method, "follow_bootstrap_room") def test_pd_decode_defaults_to_round_robin(self): - server_args = ServerArgs(model_path="dummy", disaggregation_mode="decode") + server_args = self._load_balance_args(disaggregation_mode="decode") self.assertEqual(server_args.load_balance_method, "round_robin") def test_pd_decode_radix_cache_rejects_hisparse(self): + server_args = ServerArgs( + model_path="dummy", + disaggregation_mode="decode", + disaggregation_decode_enable_radix_cache=True, + disaggregation_transfer_backend="nixl", + enable_hisparse=True, + ) with self.assertRaises(ValueError) as context: - ServerArgs( - model_path="dummy", - disaggregation_mode="decode", - disaggregation_decode_enable_radix_cache=True, - disaggregation_transfer_backend="nixl", - enable_hisparse=True, - ) + server_args._handle_pd_disaggregation() self.assertIn( "--disaggregation-decode-enable-radix-cache is incompatible with " @@ -137,8 +144,7 @@ class TestLoadBalanceMethod(unittest.TestCase): ) def test_pd_decode_radix_cache_allows_mooncake(self): - server_args = ServerArgs( - model_path="dummy", + server_args = self._load_balance_args( disaggregation_mode="decode", disaggregation_decode_enable_radix_cache=True, disaggregation_transfer_backend="mooncake", @@ -147,13 +153,14 @@ class TestLoadBalanceMethod(unittest.TestCase): self.assertFalse(server_args.disable_radix_cache) def test_pd_decode_radix_cache_rejects_fake_backend(self): + server_args = ServerArgs( + model_path="dummy", + disaggregation_mode="decode", + disaggregation_decode_enable_radix_cache=True, + disaggregation_transfer_backend="fake", + ) with self.assertRaises(ValueError) as context: - ServerArgs( - model_path="dummy", - disaggregation_mode="decode", - disaggregation_decode_enable_radix_cache=True, - disaggregation_transfer_backend="fake", - ) + server_args._handle_pd_disaggregation() self.assertIn( "--disaggregation-decode-enable-radix-cache is incompatible " @@ -162,8 +169,7 @@ class TestLoadBalanceMethod(unittest.TestCase): ) def test_pd_decode_radix_cache_allows_ascend(self): - server_args = ServerArgs( - model_path="dummy", + server_args = self._load_balance_args( disaggregation_mode="decode", disaggregation_decode_enable_radix_cache=True, disaggregation_transfer_backend="ascend", @@ -172,8 +178,7 @@ class TestLoadBalanceMethod(unittest.TestCase): self.assertFalse(server_args.disable_radix_cache) def test_pd_decode_radix_cache_allows_mooncake_tcp(self): - server_args = ServerArgs( - model_path="dummy", + server_args = self._load_balance_args( disaggregation_mode="decode", disaggregation_decode_enable_radix_cache=True, disaggregation_transfer_backend="mooncake_tcp", @@ -709,6 +714,11 @@ class TestPortArgs(unittest.TestCase): class TestSSLArgs(unittest.TestCase): + def _validate_ssl(self, **kwargs): + server_args = ServerArgs(model_path="dummy", **kwargs) + server_args._handle_ssl_validation() + return server_args + def test_default_ssl_fields_are_none(self): server_args = ServerArgs(model_path="dummy") self.assertIsNone(server_args.ssl_keyfile) @@ -718,19 +728,17 @@ class TestSSLArgs(unittest.TestCase): def test_ssl_keyfile_without_certfile_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs(model_path="dummy", ssl_keyfile="key.pem") + self._validate_ssl(ssl_keyfile="key.pem") self.assertIn("--ssl-certfile", str(context.exception)) def test_ssl_certfile_without_keyfile_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs(model_path="dummy", ssl_certfile="cert.pem") + self._validate_ssl(ssl_certfile="cert.pem") self.assertIn("--ssl-keyfile", str(context.exception)) @patch("os.path.isfile", return_value=True) def test_ssl_both_keyfile_and_certfile_accepted(self, _mock_isfile): - server_args = ServerArgs( - model_path="dummy", ssl_keyfile="key.pem", ssl_certfile="cert.pem" - ) + server_args = self._validate_ssl(ssl_keyfile="key.pem", ssl_certfile="cert.pem") self.assertEqual(server_args.ssl_keyfile, "key.pem") self.assertEqual(server_args.ssl_certfile, "cert.pem") @@ -748,9 +756,7 @@ class TestSSLArgs(unittest.TestCase): @patch("os.path.isfile", return_value=True) def test_url_returns_https_with_ssl(self, _mock_isfile): - server_args = ServerArgs( - model_path="dummy", ssl_keyfile="key.pem", ssl_certfile="cert.pem" - ) + server_args = self._validate_ssl(ssl_keyfile="key.pem", ssl_certfile="cert.pem") self.assertTrue(server_args.url().startswith("https://")) @patch("os.path.isfile", return_value=True) @@ -780,15 +786,12 @@ class TestSSLArgs(unittest.TestCase): @patch("os.path.isfile", return_value=True) def test_ssl_verify_with_ssl_no_ca(self, _mock_isfile): - server_args = ServerArgs( - model_path="dummy", ssl_keyfile="key.pem", ssl_certfile="cert.pem" - ) + server_args = self._validate_ssl(ssl_keyfile="key.pem", ssl_certfile="cert.pem") self.assertIs(server_args.ssl_verify(), False) @patch("os.path.isfile", return_value=True) def test_ssl_verify_with_ssl_and_ca(self, _mock_isfile): - server_args = ServerArgs( - model_path="dummy", + server_args = self._validate_ssl( ssl_keyfile="key.pem", ssl_certfile="cert.pem", ssl_ca_certs="ca.pem", @@ -797,18 +800,17 @@ class TestSSLArgs(unittest.TestCase): def test_ssl_ca_certs_without_certfile_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs(model_path="dummy", ssl_ca_certs="ca.pem") + self._validate_ssl(ssl_ca_certs="ca.pem") self.assertIn("--ssl-ca-certs", str(context.exception)) def test_ssl_keyfile_password_without_certfile_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs(model_path="dummy", ssl_keyfile_password="secret") + self._validate_ssl(ssl_keyfile_password="secret") self.assertIn("--ssl-keyfile-password", str(context.exception)) def test_ssl_keyfile_not_found_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs( - model_path="dummy", + self._validate_ssl( ssl_keyfile="/nonexistent/key.pem", ssl_certfile="/nonexistent/cert.pem", ) @@ -817,8 +819,7 @@ class TestSSLArgs(unittest.TestCase): def test_ssl_certfile_not_found_raises(self): with tempfile.NamedTemporaryFile(suffix=".pem") as keyfile: with self.assertRaises(ValueError) as context: - ServerArgs( - model_path="dummy", + self._validate_ssl( ssl_keyfile=keyfile.name, ssl_certfile="/nonexistent/cert.pem", ) @@ -828,8 +829,7 @@ class TestSSLArgs(unittest.TestCase): with tempfile.NamedTemporaryFile(suffix=".pem") as keyfile: with tempfile.NamedTemporaryFile(suffix=".pem") as certfile: with self.assertRaises(ValueError) as context: - ServerArgs( - model_path="dummy", + self._validate_ssl( ssl_keyfile=keyfile.name, ssl_certfile=certfile.name, ssl_ca_certs="/nonexistent/ca.pem", @@ -844,14 +844,13 @@ class TestSSLArgs(unittest.TestCase): def test_enable_ssl_refresh_without_ssl_raises(self): with self.assertRaises(ValueError) as context: - ServerArgs(model_path="dummy", enable_ssl_refresh=True) + self._validate_ssl(enable_ssl_refresh=True) self.assertIn("--enable-ssl-refresh", str(context.exception)) self.assertIn("--ssl-certfile", str(context.exception)) @patch("os.path.isfile", return_value=True) def test_enable_ssl_refresh_with_ssl_accepted(self, _mock_isfile): - server_args = ServerArgs( - model_path="dummy", + server_args = self._validate_ssl( ssl_keyfile="key.pem", ssl_certfile="cert.pem", enable_ssl_refresh=True, @@ -1160,8 +1159,7 @@ class TestPrefillOnlyDisableKvCache(unittest.TestCase): - no context-parallel attention (CP writes to the pool via set_kv_buffer), - no HiSparse (uses a different pool family), - kv_cache_dtype != fp4_e2m1 (FP4 pool is a separate allocation path). - All other configurations must be rejected at __post_init__ time so users - get a clear error before model load. + All other configurations must be rejected before model load. """ def _base_kwargs(self, **overrides): @@ -1175,47 +1173,54 @@ class TestPrefillOnlyDisableKvCache(unittest.TestCase): kwargs.update(overrides) return kwargs + def _validate_prefill_only_args(self, **overrides): + sa = ServerArgs(**self._base_kwargs(**overrides)) + sa._handle_legacy_cp_arguments() + sa._validate_prefill_only_disable_kv_cache_args() + return sa + def test_valid_minimal_config_constructs(self): - sa = ServerArgs(**self._base_kwargs()) + sa = self._validate_prefill_only_args() self.assertTrue(sa.prefill_only_disable_kv_cache) def test_rejects_when_not_embedding(self): with self.assertRaisesRegex(ValueError, "requires --is-embedding"): - ServerArgs(**self._base_kwargs(is_embedding=False)) + self._validate_prefill_only_args(is_embedding=False) def test_rejects_when_chunked_prefill_size_not_minus_one(self): with self.assertRaisesRegex(ValueError, "--chunked-prefill-size=-1"): - ServerArgs(**self._base_kwargs(chunked_prefill_size=8192)) + self._validate_prefill_only_args(chunked_prefill_size=8192) def test_rejects_when_radix_cache_enabled(self): with self.assertRaisesRegex(ValueError, "--disable-radix-cache"): - ServerArgs(**self._base_kwargs(disable_radix_cache=False)) + self._validate_prefill_only_args(disable_radix_cache=False) def test_rejects_attn_cp_size_greater_than_one(self): with self.assertRaisesRegex(ValueError, "--attn-cp-size"): - ServerArgs(**self._base_kwargs(attn_cp_size=2, tp_size=2)) + self._validate_prefill_only_args(attn_cp_size=2, tp_size=2) def test_rejects_prefill_context_parallel(self): with self.assertRaisesRegex(ValueError, "--enable-prefill-cp"): - ServerArgs(**self._base_kwargs(enable_prefill_context_parallel=True)) + self._validate_prefill_only_args(enable_prefill_context_parallel=True) def test_rejects_hisparse(self): with self.assertRaisesRegex(ValueError, "--enable-hisparse"): - ServerArgs(**self._base_kwargs(enable_hisparse=True)) + self._validate_prefill_only_args(enable_hisparse=True) def test_rejects_fp4_kv_cache(self): with self.assertRaisesRegex(ValueError, "fp4_e2m1"): - ServerArgs(**self._base_kwargs(kv_cache_dtype="fp4_e2m1")) + self._validate_prefill_only_args(kv_cache_dtype="fp4_e2m1") class TestSessionRadixCacheServerArgs(unittest.TestCase): def test_requires_priority_radix_eviction_policy(self): + server_args = ServerArgs( + model_path="dummy", + enable_session_radix_cache=True, + radix_eviction_policy="lru", + ) with self.assertRaisesRegex(ValueError, "--radix-eviction-policy priority"): - ServerArgs( - model_path="dummy", - enable_session_radix_cache=True, - radix_eviction_policy="lru", - ) + server_args._handle_cache_compatibility() class TestCudaGraphConfigDataclassAccess(CustomTestCase):