Skip to content

CP-52708: Avoid making Unix read/write calls for internal API calls: …#6178

Closed
edwintorok wants to merge 2 commits intoxapi-project:masterfrom
edwintorok:pr/CP-52708
Closed

CP-52708: Avoid making Unix read/write calls for internal API calls: …#6178
edwintorok wants to merge 2 commits intoxapi-project:masterfrom
edwintorok:pr/CP-52708

Conversation

@edwintorok
Copy link
Copy Markdown
Member

…forward the API call directly like we do with the CLI for calls to the coordinator when we are the coordinator

We might want to check that tracing still works with this, e.g. we might have to create a new span.

This avoids a lot of needless internal serialization, and a similar mechanism is already used by the CLI.

Comment thread ocaml/xapi/xapi.ml Outdated
@edwintorok edwintorok changed the base branch from feature/perf to master February 4, 2025 17:40
Comment thread ocaml/xapi/helpers.ml
This will enable short-circuiting internal API calls.

When the FD is missing mark the call as Internal.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
…forward the API call directly like we do with the CLI for calls to the coordinator when we are the coordinator

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
@edwintorok edwintorok marked this pull request as ready for review February 6, 2025 18:45
@edwintorok edwintorok enabled auto-merge February 6, 2025 18:48
@edwintorok edwintorok marked this pull request as draft February 6, 2025 18:49
auto-merge was automatically disabled February 6, 2025 18:49

Pull request was converted to draft

@edwintorok
Copy link
Copy Markdown
Member Author

Moving back to draft, no feature flag on this one.

@psafont
Copy link
Copy Markdown
Member

psafont commented Jun 16, 2025

This seems quite safe and contained to merge, are you sure this should still be a draft?

@lindig
Copy link
Copy Markdown
Contributor

lindig commented Jun 18, 2025

Unrelated: why is a parameter that is used everywhere using a long and unusual name like __context? Rather than ctx or similar? What are the underscores achieving?

@robhoes
Copy link
Copy Markdown
Member

robhoes commented Jun 18, 2025

@lindig I think you need a time machine set to 2006 in order to answer that question.

@psafont
Copy link
Copy Markdown
Member

psafont commented Jun 18, 2025

I can try a quick grep + sed if it makes you happy, I would be painful to rebase all work being done though 😬

@lindig
Copy link
Copy Markdown
Contributor

lindig commented Jun 26, 2025

Can we merge this?

@robhoes
Copy link
Copy Markdown
Member

robhoes commented Oct 20, 2025

Looking at this again, I wonder if/how it affects tracing and also logging. Would we lose info like the source IP from the log lines?

Copy link
Copy Markdown
Contributor

@GabrielBuica GabrielBuica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is affecting distributed tracing. Might be worth a check.

It seems to me that this is pretty straight forward and has been lingering for a while.
Maybe run a few tests with dt enabled, check logs and merge it.
Or alternatively closed if it goes nowhere.

@edwintorok edwintorok closed this Mar 13, 2026
@edwintorok edwintorok deleted the pr/CP-52708 branch March 13, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants