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

Header-only response will not trigger resp(rpc) handling #37

Open
jokerwyt opened this issue Sep 5, 2024 · 0 comments
Open

Header-only response will not trigger resp(rpc) handling #37

jokerwyt opened this issue Sep 5, 2024 · 0 comments

Comments

@jokerwyt
Copy link
Contributor

jokerwyt commented Sep 5, 2024

  • Backend: envoy native
# circuitbreaker.appnet
state:
    max_concurrent_req: int
    pending_req: int
    drop_count: int

init():
    max_concurrent_req = 1
    pending_req = 0
    drop_count = 0

req(rpc):
    match pending_req <= max_concurrent_req:
	true =>
            pending_req = pending_req + 1
	    send(rpc, Down)
	false =>
            drop_count = drop_count + 1
	    send(err('circuit breaker'), Up)

resp(rpc):
    pending_req = pending_req - 1
    send(rpc, Up)

We put the whole resp() into Envoy filter's encodeData(). When there is a header-only response, the response handling will NOT be executed as expected.

Our current server will not return header-only responses for now. But if we have chain like circuitbreaker -> firewall, when the firewall rejects the request, the circuitbreaker will receive a header-only response. Then it will skip the resp handling that should have been executed, which makes pending_req not consistent.

Possible solutions

As background, our current appnet program semantics are:

if we send an error in req() in THIS element, we do not execute resp handling. But if we normally pass the request downstream, we should. (even the later elements return error directly) .

(But some of our appnet programs mistakenly assume we are always handling successful responses in resp, which will be another problem to fix.)

We have two directions:

  1. Separate header and body handling of resp(). This is closer to Envoy Filter itself and it demands more verbose handling.
  2. Find a way to detect header-only responses in runtime, and do resp() things in encodeHeader if so. This will introduce minimal engineering effort. The error caused by access to body data when impossible is left to users.
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

No branches or pull requests

1 participant