Rename DSpark hidden protocol to PD hidden - #648
zhangxiaolei123456 merged 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to rename references of the 'DSpark' hidden protocol to 'PD' hidden protocol across various modules, including connection managers, schedulers, and model executors. The review feedback points out several missed renames that should be addressed for consistency, including endpoint string literals, methods like send_dspark_owner_direct_hidden_for_batch and _pp_maybe_send_dspark_owner_direct_hidden, and variables such as dspark_layers_to_capture and capture_dspark.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| self.pd_hidden_ack_completion_cv = threading.Condition() | ||
| self.pd_hidden_acked_chunks: Dict[int, List[dict]] = defaultdict(list) | ||
| self.pd_hidden_acked_lock = threading.Lock() | ||
| self.pd_hidden_ack_wakeup_endpoint = ( |
There was a problem hiding this comment.
The endpoint string literal still contains dspark-hidden-ack- while the variable name was renamed to self.pd_hidden_ack_wakeup_endpoint. For consistency with the renaming of the DSpark hidden protocol to PD hidden, this should be updated to pd-hidden-ack-.
| written[local_start:local_end] = [True] * rows | ||
| if send_owner_direct: | ||
| self._send_dspark_hidden_only_chunk(req) | ||
| self._send_pd_hidden_only_chunk(req) |
There was a problem hiding this comment.
The method send_dspark_owner_direct_hidden_for_batch was not renamed to send_pd_owner_direct_hidden_for_batch. For consistency with the renaming of the DSpark hidden protocol to PD hidden, please rename this method and all its references.
| result: GenerationBatchResult, | ||
| ) -> None: | ||
| if not self._pp_should_owner_direct_dspark_hidden(batch): | ||
| if not self._pp_should_owner_direct_pd_hidden(batch): |
There was a problem hiding this comment.
The method _pp_maybe_send_dspark_owner_direct_hidden and the reference to send_dspark_owner_direct_hidden_for_batch were not renamed to use pd instead of dspark. For consistency, please rename them to _pp_maybe_send_pd_owner_direct_hidden and send_pd_owner_direct_hidden_for_batch respectively.
|
|
||
| dspark_layers_to_capture = getattr( | ||
| forward_batch, "dspark_hidden_capture_layer_ids", None | ||
| forward_batch, "pd_hidden_capture_layer_ids", None |
There was a problem hiding this comment.
The variable dspark_layers_to_capture was not renamed to pd_layers_to_capture. For consistency with the renaming of the DSpark hidden protocol to PD hidden, please rename this variable.
| if capture_dspark: | ||
| for idx, aux_hidden in enumerate(dspark_aux_hidden_states): | ||
| proxy_tensors[f"dspark_aux_hidden_states_{idx}"] = ( | ||
| for idx, aux_hidden in enumerate(pd_aux_hidden_states): |
There was a problem hiding this comment.
The variable capture_dspark was not renamed to capture_pd. For consistency with the renaming of the DSpark hidden protocol to PD hidden, please rename this variable.
f334074
into
fix/dspark-review-dedupe-current
1 participant
Motivation
Modifications
Accuracy Tests
Speed Tests and Profiling
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ❌ Run #29751733102
Latest PR Test (Extra): ❌ Run #29751732637