Periodically send empty report to detect rollout changes#769
Periodically send empty report to detect rollout changes#769qiwzhang merged 3 commits intocloudendpoints:masterfrom
Conversation
Signed-off-by: Wayne Zhang <qiwzhang@google.com>
| ReportResponse* response = new ReportResponse; | ||
| Call(request, response, | ||
| [this, response](const ::google::protobuf::util::Status&) { | ||
| delete response; |
There was a problem hiding this comment.
I'm confused: Where does ESP read the latest rollout ID from the ReportResponse? Shouldn't it be using the response to check for it?
There was a problem hiding this comment.
Just using the existing code to process the Check/Report response. It is in the HandleRespones() function.
There was a problem hiding this comment.
This change only adds timer to trigger a dummy Report calls in every minutes. The existing code will handle the rest: on every check/response code it will check rollout id, then it set to a callback function which is intercepted by config_manager.
TAOXUY
left a comment
TAOXUY
left a comment
There was a problem hiding this comment.
Since right now we don't polling on servicemanagement, I suggest we change the logic from timer trigger to be a new rollout trigger(in the callback). The benefit would be the new rollout can be updated soon. It can be 60s earlier at the edge case if the servicemangement timer is triggered just before the empty report timer and new rollout fetched by empty report has to wait extra 60s. Same for real report update.
Also, maybe add unit test to cover this change.
|
Hi @TAOXUY, I did not quite understand what you mean by rollout trigger. Could you elaborate? |
Here, a new fetch depends on 1. new config rollout id 2. the timer fire. Before, we cannot rely on the update of real report if no requests coming. Now, since 1. is relied on another timer by sending empty report request, we really don't need 2. and we can just fetch the new config. |
|
@TAOXUY Here, this timer is for throttle purpose. It is AFTER detected rollout change, but not to call ServiceManagment at the same time, we need to throttle it in a 5 minute window so different proxy instance will call it at different time within that window to make sure Inception can handle the load. We still need that throttle features. |
I see. IMHO, now the polling(checking latest rollout) pressure is moved from servicemanagement to servicecontrol, is it still necessary to throttle? |
|
Yes, fetching service config is from ServiceManagement, we should not add too much load to that service. We still need to throttle it. |
3 participants
Signed-off-by: Wayne Zhang qiwzhang@google.com
Sending an empty report to ServiceControl will be replied by the latest rollout ID.
Using this way to detect if service config rollout has been updated.
Update the e2e test, by not sending traffic to test if config has been updated.