Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pageserver_client::mgmt_api: use TenantShardId instead of TenantId #6154

Closed
problame opened this issue Dec 15, 2023 · 3 comments · Fixed by #7313
Closed

pageserver_client::mgmt_api: use TenantShardId instead of TenantId #6154

problame opened this issue Dec 15, 2023 · 3 comments · Fixed by #7313
Assignees
@jcsp
Labels
c/storage/pageserver Component: storage: pageserver m/good_first_issue Moment: when doing your first Neon contributions

Comments

@problame
Copy link
Contributor 

          Noticed that some of the methods should be taking `TenantShardId` instead of `TenantId`, but, none of the callers seem to be sharding-aware. So, leaving that for another time. cc @jcsp

Originally posted by @problame in #6127 (comment)
@problame problame added c/storage/pageserver Component: storage: pageserver m/good_first_issue Moment: when doing your first Neon contributions labels Dec 15, 2023
@problame problame mentioned this issue Dec 15, 2023
Merged
problame added a commit that referenced this issue Dec 15, 2023
@problame
add a Rust client for Pageserver management API (#6127)
1a9854b 
Part of getpage@lsn benchmark epic:
#5771

This PR moves the control plane's spread-all-over-the-place client for
the pageserver management API into a separate module within the
pageserver crate.

I need that client to be async in my benchmarking work, so, this PR
switches to the async version of `reqwest`.
That is also the right direction generally IMO.

The switch to async in turn mandated converting most of the
`control_plane/` code to async.

Note that some of the client methods should be taking `TenantShardId`
instead of `TenantId`, but, none of the callers seem to be
sharding-aware.
Leaving that for another time:
#6154
@ScottLinnn
Copy link

I'd like to have a try on this!

@jcsp
Copy link
Collaborator

jcsp commented Jan 4, 2024

@ScottLinnn most of these are updated in #6251, but if you would like to open a PR to update them all in one commit, that would be cleaner, so feel free.

@ScottLinnn
Copy link

Thanks for letting me know! I'd prefer looking at other issues.

@jcsp jcsp self-assigned this Apr 4, 2024
@jcsp jcsp mentioned this issue Apr 4, 2024
Merged
5 tasks
@jcsp jcsp closed this as completed in #7313 Apr 4, 2024
jcsp added a commit that referenced this issue Apr 4, 2024
@jcsp
pageserver: update mgmt_api to use TenantShardId (#7313)
e17bc6a 
## Problem

The API client was written around the same time as some of the server
APIs changed from TenantId to TenantShardId

Closes: #6154

## Summary of changes

- Refactor mgmt_api timeline_info and keyspace methods to use
TenantShardId to match the server

This doesn't make pagebench sharding aware, but it paves the way to do
so later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jcsp jcsp

Labels
c/storage/pageserver Component: storage: pageserver m/good_first_issue Moment: when doing your first Neon contributions
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pageserver: update mgmt_api to use TenantShardId neondatabase/neon
3 participants
@jcsp @problame @ScottLinnn