merlimat commented on PR #25822:
URL: https://github.com/apache/pulsar/pull/25822#issuecomment-4502652953

   Thanks for the thorough review @lhotari. Addressed in 74fab13:
   
   **Naming** — renamed "special segment" → "legacy segment" 
(`legacyTopicName`, `isLegacy()`), per the inline thread. A legacy segment is 
one not managed by the scalable-topic controller; it wraps an existing, 
externally-managed `persistent://` topic.
   
   **Bugs**
   1. **`toScalableTopic()` partition suffix** — now strips `-partition-K` (via 
`getPartitionedTopicName()`), so `persistent://t/n/x-partition-3` resolves to 
`topic://t/n/x`. Covered by `TopicNameTest.testToScalableTopic`.
   2. **Partition-name input chaining** — `DagWatchSession.start()` now rejects 
an individual-partition lookup target up front. Covered by 
`testStartRejectsIndividualPartitionInput`.
   3. **`partitions > 65536`** — synthetic ranges are cosmetic (routing is 
mod-N over `segment_id`), so past the 16-bit hash space a degenerate clamped 
slot is used instead of letting `width` hit 0 and violating the `HashRange` 
invariant.
   5. **`isLegacy()` empty string** — now `!= null && !isEmpty()` (both broker 
and SDK side).
   6. **`newScalableTopicUpdate(null, ...)`** — guards the optional 
`resolved_topic_name` setter against null. Covered by a new 
`CommandsScalableTopicTest` case.
   
   **Quality / docs**
   4. **Synthetic layout for non-existent topics** — confirmed intentional and 
documented: existence/auto-create is decided downstream by the namespace policy 
when the producer/consumer attaches, matching v4 semantics.
   7. **Cosmetic hash ranges** — documented that synthetic ranges are never 
consulted for routing (mod-N over `segment_id`).
   10. **Short-form input** — now exercised in `testToScalableTopic`.
   
   **Confirmed non-issue**
   8. **V1 names** — `TopicName.get()` rejects V1 names (with a cluster 
component) before `toScalableTopic()` can see them; existing 
`testToFullTopicName` already asserts this.
   
   **Deferred**
   9. **`pushUpdate` race** — pre-existing, not introduced here; tracking 
separately.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to