Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

potential delay between recieving a response and firing event hooks #285

Open
blampe opened this issue Nov 2, 2015 · 1 comment
Open

Comments

@blampe
Copy link
Contributor

blampe commented Nov 2, 2015

I'm skeptical of the events we're seeing reported. Right now response logic looks like this:

response_future = connection.send_request(req)

with timeout(response_future, req.ttl):
    try:
        response = yield response_future
    except TChannelError as error:
        # event: after_receive_error
        self.tchannel.event_emitter.fire(
            EventType.after_receive_error, req, error,
        )
        raise
# event: after_receive_response
self.tchannel.event_emitter.fire(
    EventType.after_receive_response, req, response,
)

I'm pretty sure this is wrong because the yield statement isn't guaranteed to resume immediately after the future resolves, so there's potential for delay between receiving a response and firing the after_receive_response event for it.

More correct (I think) would be this:

response_future = connection.send_request(req)

def fire_events(f):
    if f.exception():
        self.tchannel.event_emitter.fire(
            EventType.after_receive_error, req, error,
        )
        raise f.exception()
    self.tchannel.event_emitter.fire(
        EventType.after_receive_response, req, response,
    )

response_future.add_done_callback(fire_events)

with timeout(response_future, req.ttl):
        yield response_future

because add_done_callback is preemptive and will execute as soon as the future resolves.

@HelloGrayson HelloGrayson mentioned this issue Nov 2, 2015
21 tasks
@jc-fireball
Copy link
Contributor

It's true the sequence of callback is add_done_callback > add_future > yield

@abhinav abhinav mentioned this issue Nov 9, 2015
23 tasks
@abhinav abhinav mentioned this issue Dec 7, 2015
19 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants