-
Notifications
You must be signed in to change notification settings - Fork 2
Admin API: add missing /v1/cloud_storage/ endpoints #45
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
base: main
Are you sure you want to change the base?
Admin API: add missing /v1/cloud_storage/ endpoints #45
Conversation
📝 WalkthroughWalkthroughThe change updates the Admin API specification to introduce cloud storage recovery and cache management functionality. It adds 6 new endpoints for automated recovery status monitoring, cache trimming, and metadata management operations. The specification also introduces 4 new public schemas ( Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
|
ℹ️ API content change detected: No structural change, nothing to display.
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
admin/admin.yaml (5)
2331-2332: Consider returning a typed response body instead of emptycontent: {}for “sync_local_state” 200.
Right now 200 has no schema, which makes it hard for docs/clients to know what to expect (even a minimal{ status: ... }).
2332-2362:/v1/cloud_storage/automated_recovery: request body is an unstructured empty object.
If this endpoint truly takes no input, consider removingrequestBodyentirely (or define explicit properties if there are known knobs), to reduce ambiguity in generated clients.
2363-2386:/v1/cloud_storage/cache/trim: clarify parameter semantics and bounds (bytes/objects).
Right nowbytes/objectsare plain integers with nominimumand no note on what happens if both are set (AND vs precedence). Adding that detail avoids misinterpretation.
5099-5114:metadata_anomaly.previous_segment: avoidtype: objectwithallOf.
Some OpenAPI tooling rejects/ignorestypewhenallOfis present. Prefer justnullable: true+allOf: [$ref...](or useoneOfif it can be eithersegment_metaor null).
5115-5161:cloud_storage_partition_anomalies: consider defining item schemas formissing_*arrays.
missing_spillover_manifests/missing_segmentsarearray<string>; if these are paths/keys, consider naming/format hints (e.g.,description,example, orformat) to improve docs usability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
admin/admin.yaml(4 hunks)
| post: | ||
| tags: | ||
| - Tiered Storage | ||
| summary: Reset metadata from cloud | ||
| description: Resets the manifest to the one in cloud storage, updating all replicas with the given manifest. The request is refused if applying the change would cause data loss and the force query parameter is unspecified or false. | ||
| operationId: unsafe_reset_metadata_from_cloud | ||
| parameters: | ||
| - name: namespace | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| - name: topic | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| - name: partition | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: integer | ||
| - name: force | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: boolean | ||
| responses: | ||
| 200: | ||
| description: Partition metadata is reset | ||
| content: {} | ||
| /v1/cloud_storage/reset_scrubbing_metadata/{namespace}/{topic}/{partition}: |
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.
unsafe_reset_metadata_from_cloud: document refusal/error responses and make safety explicit.
The description says the request “is refused” when force is unspecified/false, but only a 200 response is documented. Consider adding explicit non-2xx responses (e.g., 409/422 with an error schema) and a stronger warning in summary/description since this is a destructive/high-risk operation.
🤖 Prompt for AI Agents
In admin/admin.yaml around lines 2417 to 2448, the
unsafe_reset_metadata_from_cloud operation only documents a 200 response but its
description states the request is “refused” when force is unspecified/false;
update the OpenAPI operation to explicitly document non-2xx error responses
(e.g., 409 Conflict or 422 Unprocessable Entity) that represent refusal or
potential data-loss errors and reference a standard error response schema (or
create one under components/schemas). Also strengthen the summary/description
with an explicit warning about destructive behavior and clarify that callers
must set force=true to override, and include examples or error messages in the
new response entries to make failure cases clear for clients.
| stm_region_size_bytes: | ||
| type: integer | ||
| description: Total size of the addressable segments in the STM region of the log | ||
| archive_size_bytes: | ||
| type: integer | ||
| description: Total size of the archive region of the log | ||
| local_log_size_bytes: | ||
| type: integer | ||
| description: Total size of the addressable local log for the partition | ||
| stm_region_segment_count: | ||
| type: integer | ||
| description: Number of segments in the STM region of the cloud log | ||
| cloud_log_segment_count: | ||
| type: integer | ||
| description: Number of segments in the STM region of the cloud log | ||
| local_log_segment_count: | ||
| type: integer | ||
| description: Number of segments in the local log | ||
| cloud_log_start_offset: | ||
| type: integer | ||
| nullable: true | ||
| description: The first Kafka offset accessible from the cloud (inclusive) | ||
| stm_region_start_offset: | ||
| type: integer | ||
| nullable: true | ||
| description: The first Kafka offset accessible from the cloud in the STM region (inclusive) | ||
| cloud_log_last_offset: |
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.
Fix cloud_log_segment_count description (currently duplicates STM-region wording).
With stm_region_segment_count added, cloud_log_segment_count’s description reading “Number of segments in the STM region…” is confusing/likely incorrect. Suggest aligning it to “Number of segments in the addressable cloud log” (or whatever the intended meaning is).
🤖 Prompt for AI Agents
In admin/admin.yaml around lines 5004 to 5030, the description for
cloud_log_segment_count incorrectly duplicates STM-region wording; update that
description to reflect the cloud log (e.g., "Number of segments in the
addressable cloud log") so it aligns with stm_region_segment_count and other
cloud_* fields, keeping wording consistent with the surrounding fields.
This PR adds missing endpoints and properties from https://github.com/redpanda-data/redpanda/blob/dev/src/v/redpanda/admin/api-doc/shadow_indexing.json, including
/v1/cloud_storage/automated_recoverywhich needs to be referenced in redpanda-data/docs#1513.New Tiered Storage Endpoints:
/v1/cloud_storage/automated_recoveryendpoints for initializing and checking the status of cluster recovery./v1/cloud_storage/cache/trimendpoint to allow trimming of the local tiered storage cache with optional parameters for objects and bytes./v1/cloud_storage/anomalies/{namespace}/{topic}/{partition}endpoint to retrieve cloud storage anomalies for a specific partition./v1/cloud_storage/unsafe_reset_metadata_from_cloud/{namespace}/{topic}/{partition}endpoint to reset partition metadata from the cloud, with safety checks to prevent data loss./v1/cloud_storage/reset_scrubbing_metadata/{namespace}/{topic}/{partition}endpoint to reset scrubbing metadata and anomalies for a partition.Schema Enhancements for Cloud Storage Metadata:
stm_region_size_bytes,archive_size_bytes,stm_region_segment_count,stm_region_start_offset, and improved descriptions for segment counts.segment_metaschema to describe uploaded segment metadata, including offsets, timestamps, size, compaction, and terms.metadata_anomalyandcloud_storage_partition_anomaliesschemas to provide detailed reporting of detected anomalies, missing manifests, missing segments, and anomaly discard counts.Cluster Recovery Status Reporting:
cluster_recovery_statusschema to report the state and error (if any) of cluster recovery operations.Minor Changes and Cleanups:
x-codegen-request-body-name: bodyfrom an existing endpoint definition.