[improve][broker] getPartitionedStats should return 404 instead of empty stats when topic is not loaded#26041
Open
zjxxzjwang wants to merge 1 commit into
Conversation
…pty stats when topic is not loaded
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
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.
Motivation
When calling the admin REST API
getPartitionedStatsfor a partitioned topic, if the topic's partitions have not been loaded yet (i.e. none of the partitions has amanaged-ledgersznode created in metadata store), the current implementation returns an HTTP200 OKwith an "empty"PartitionedTopicStatsImplobject (all counters are zero,partitionsmap is empty or only contains a placeholderTopicStatsImpl).This behavior is inconsistent with the non-partitioned counterpart
getStats, which correctly returns404 NOT_FOUNDin the same situation. As a result:The two endpoints should behave consistently: when the underlying topic data is not present, both should return
404.Modifications
In
PersistentTopicsBase#internalGetPartitionedStats:FutureUtil.waitForAll(topicStatsFutureList)completes and we iterate over each per-partition stats future, count how many partitions actually returned stats successfully (successCount). A partition whosemanaged-ledgersznode does not exist will fail itsgetStatsAsynccall (404 from the per-partition path), so its future ends upcompletedExceptionallyand is skipped — exactly the case we want to detect.successCount == 0(i.e. no partition produced stats), resume the response withRestException(NOT_FOUND, getPartitionedTopicNotFoundErrorMessage(topicName)), the same error path used bygetStats. This aligns the 404 semantics and error message between the two APIs.if (perPartition && stats.partitions.isEmpty())fallback branch that previously calledpartitionedTopicExistsAsyncand either inserted a placeholder emptyTopicStatsImplor returned"Internal topics have not been generated yet". With the newsuccessCount == 0check placed earlier, this branch becomes dead code: wheneversuccessCount > 0andperPartition == true,stats.partitionsis guaranteed to be non-empty (theputhappens in the sameifblock assuccessCount++).The change is minimal and only touches the result-aggregation step inside
internalGetPartitionedStats; the per-partition fetching logic (isServiceUnitOwnedAsync→ localasyncGetStats/ remote admin call) is unchanged.Verifying this change
This change is already covered by existing tests for
getStats404 behavior on partitioned topics; the new branch reuses the exact same error message (getPartitionedTopicNotFoundErrorMessage) and HTTP status, so existing assertions that rely on404for unloaded topics now also apply to the partitioned-stats endpoint.Does this pull request potentially affect one of the following parts:
Highlight on REST endpoints
GET /admin/v2/persistent/{tenant}/{namespace}/{topic}/partitioned-statsnow returns404 NOT_FOUNDwith message"Partitioned Topic not found: <topic> has zero partitions"(the standardgetPartitionedTopicNotFoundErrorMessage) when no partition of the topic has been loaded, instead of returning200 OKwith an empty stats object.Consumers of this endpoint that previously relied on receiving an empty-but-successful response for unloaded topics will need to handle
404— which is already the expected behavior of the non-partitionedgetStatsAPI.