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.

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

connmgr: add ability to respond via router instead of pub #48078

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

jkarneges
Copy link
Member

@jkarneges jkarneges commented Oct 2, 2024

Related to #48073, this completes the implementation in connmgr for sending response data via router. After this, proxy will need to be updated to support receiving response data this way.

Notably, the following changes are made:

  1. make_zhttp_response() is reworked to support producing either PUB-style or ROUTER-style message formats, returning a Some(addr) in the case of the latter. It is also made public and shared across modules to reduce code duplication.
  2. If router-resp is indicated on the request, a flag is set to true on the StreamSharedData struct. Whenever response data needs to be sent, this flag can be checked to decide whether to respond via ROUTER or not.
  3. When adding to a batch, specify whether router mode should be used.
  4. Instead of always specifying None for the address, the value is specified as Some or None depending on whether router-resp was requested.

The rest of the changes are mainly tests.

@jkarneges jkarneges force-pushed the jkarneges/allow-router-resp branch from 61d214c to 301c8a8 Compare October 3, 2024 21:14
@@ -420,46 +425,30 @@ impl Connections {

assert!(count <= zhttppacket::IDS_MAX);

let zreq = zhttppacket::Request {
let zresp = zhttppacket::Response {
Copy link
Member Author

@jkarneges jkarneges Oct 3, 2024

Choose a reason for hiding this comment

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

This was supposed to be a response packet rather than a request packet. We've simply been lucky that they serialize the same.

@jkarneges jkarneges marked this pull request as ready for review October 3, 2024 21:34
@jkarneges jkarneges merged commit 2fff730 into main Oct 4, 2024
19 checks passed
@jkarneges jkarneges deleted the jkarneges/allow-router-resp branch October 4, 2024 16:59
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.

2 participants