server: enable _status/ranges API for secondary tenants#131100
server: enable _status/ranges API for secondary tenants#131100craig[bot] merged 1 commit intocockroachdb:masterfrom
_status/ranges API for secondary tenants#131100Conversation
|
This change is |
shubhamdhama
left a comment
There was a problem hiding this comment.
Keeping it draft as there are few TODOs I need to clear.
There was a problem hiding this comment.
cockroach/pkg/server/status.go
Lines 2663 to 2667 in 6a5017e
Here we take next for local TenantRanges but ignoring next for remote responses. I think this pagination stuff is incomplete/incorrect for TenantRanges endpoint. But more importantly pagination is not used so in a follow-up we can consider removing this. I almost removed it myself but reverted the change as I'm not an expert in this area. Let me know if removing it would make others happy as well.
There was a problem hiding this comment.
Feel free to put up a separate PR removing the pagination, I would not combine with this work. I'm not super familiar with the usage of this endpoint but I agree the pagination here is confusing at first glance. What does it mean to send the same limit/offset to all the nodes during fanout? Are their lists guaranteed to be the same size?
The next is ignored for remote responses because it comes back already inside the resp.
954b5e9 to
0157e73
Compare
0157e73 to
a66741c
Compare
There was a problem hiding this comment.
Feel free to put up a separate PR removing the pagination, I would not combine with this work. I'm not super familiar with the usage of this endpoint but I agree the pagination here is confusing at first glance. What does it mean to send the same limit/offset to all the nodes during fanout? Are their lists guaranteed to be the same size?
The next is ignored for remote responses because it comes back already inside the resp.
Previously, the above endpoint was only available for the system tenant. With this change, it now works for secondary tenants and returns the ranges belonging to the currently logged-in tenant. Note that although we have `TenantRanges`, which is quite similar to `Ranges`, it was developed to meet the needs of the `debug` command and has a different response body. Another difference is that `TenantRanges` returns ranges for all nodes, while `Ranges` returns data for a particular node. Fixes: cockroachdb#110019 Epic: CRDB-38968 Release note (multi-tenancy): The `ranges` endpoint on the `Advanced Debug` section of the DB Console is now enabled for secondary tenants and will return the ranges only for the currently logged-in tenant. For the system tenant, it will continue to work as before, returning ranges for all tenants on the specified node.
a66741c to
8a35ea7
Compare
|
Thanks for the review! bros r=dhartunian |
|
👊 |
|
OH-kayy, I misspelled! bors r=dhartunian |
Previously, the above endpoint was only available for the system tenant. With this change, it now works for secondary tenants and returns the ranges belonging to the currently logged-in tenant.
Note that although we have
TenantRanges, which is quite similar toRanges, it was developed to meet the needs of thedebugcommand and has a different response body. Another difference is thatTenantRangesreturns ranges for all nodes, whileRangesreturns data for a particular node.Fixes: #110019
Epic: CRDB-38968
Release note (multi-tenancy): The
rangesendpoint on theAdvanced Debugsection of the DB Console is now enabled for secondary tenants and will return the ranges only for the currently logged-in tenant. For the system tenant, it will continue to work as before, returning ranges for all tenants on the specified node.