Skip to content

Removed preemptive RPC timeout.#193

Open
TimSimpsonR wants to merge 1 commit intohub-cap:masterfrom
TimSimpsonR:be-gone-fake-rpc-timeout
Open

Removed preemptive RPC timeout.#193
TimSimpsonR wants to merge 1 commit intohub-cap:masterfrom
TimSimpsonR:be-gone-fake-rpc-timeout

Conversation

@TimSimpsonR
Copy link
Copy Markdown

  • Took out the RPC timeout based on the agent heartbeat in the database.
    This isn't sure-fire and thus might harm guest communications.
  • Added better log messages during RPC timeouts which state the method
    which has timed out.

* Took out the RPC timeout based on the agent heartbeat in the database.
  This isn't sure-fire and thus might harm guest communications.
* Added better log messages during RPC timeouts which state the method
  which has timed out.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's worse than that. He's dead, Jim!

@ed-
Copy link
Copy Markdown

ed- commented Sep 17, 2012

+1

@rnirmal
Copy link
Copy Markdown

rnirmal commented Sep 17, 2012

You might want to think this over, before removing it. The reason it was added is so that the API calls don't hang forever if the agent is down or the instance stops responding etc.

@TimSimpsonR
Copy link
Copy Markdown
Author

I'd emailed the team about this and no one knew why we had that code, so thanks for the update. We already have RPC timeouts so I don't understand what this adds. If its worthwhile in fact we should add it to all calls. My problem with it is if the clock time gets skewed somehow (which would be a bug anyway, but still happens all the time in my VM) it will shut off communication to all the guest for no reason.

@cp16net
Copy link
Copy Markdown

cp16net commented Sep 20, 2012

i'm all for the extra logging. the heartbeat part i am a little uneasy about removing it all together although i understand that it was only used in a single place/method. I think that might have been because as we added new features it was unclear what checking the heart beat would mean. I am speculating here but if it buys us an error faster than waiting for a timeout for guest agent calls that might not be a bad thing when our system grows.

@cp16net
Copy link
Copy Markdown

cp16net commented Sep 26, 2012

+1

1 similar comment
@imsplitbit
Copy link
Copy Markdown

+1

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.

5 participants