-
Notifications
You must be signed in to change notification settings - Fork 39
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
Admin API endpoints for nodewise recovery #194
Conversation
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
summary: Get partitions with lost majority | ||
description: List of partitions with lost majority given an input set of dead nodes. | ||
operationId: majority_lost | ||
# responses: |
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.
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.
Correct, "void" is a mistake, this endpoint returns an array of ntp_with_majority_loss
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.
@ztlpn can we update the spec to be accurate please? This will be important because we are trying to automate Admin API doc generation from the spec moving forward. Thanks
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.
Paging @bharathv as the main author :)
Although I would say that problems with keeping schemas for request bodies in sync with the code are bound to appear because they these schemas are not used anywhere (we do separate validation in the admin server code).
in: query | ||
required: true | ||
schema: | ||
type: string |
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.
@bharathv this spec indicates query parameter for this API call. The force recover operation requires an input array of nodes in the API request body. Just want to double check that both are correct. Does the request URL look something like /partitions/majority_lost?dead_nodes=1,3,5
?
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.
/partitions/majority_lost?dead_nodes=1,3,5
yes you are correct, thats how the URL looks. A GET with a request_body is not considered a standard practice in the implementations, so I had to use a query parameter.
redpanda-data/redpanda#13943 (comment)
58f3de0
to
ded8d0c
Compare
What's the latest status of this PR @kbatuigas ? |
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.
@JakeSCahill should this work to add a "slot" for this particular method? I see a slot element show up in the HTML source but it doesn't actually render.
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.
I added a fix here ea0d3ea
The syntax is {method}-{path}
for the directory name (without the curly braces).
In order to support this, I needed to adjust our UI here: redpanda-data/docs-ui#217
You won't see the slot until the UI change is merged.
in: query | ||
required: true | ||
schema: | ||
type: string |
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.
/partitions/majority_lost?dead_nodes=1,3,5
yes you are correct, thats how the URL looks. A GET with a request_body is not considered a standard practice in the implementations, so I had to use a query parameter.
redpanda-data/redpanda#13943 (comment)
Preview - GET /partitions/majority_lost
Preview - POST /partitions/force_recover_from_nodes
This feature was released in 23.3 and the main (conceptual) doc was merged through this PR: https://github.com/redpanda-data/documentation-private/issues/1982.