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

Log requests to GCE #26170

Merged
merged 1 commit into from
May 24, 2016
Merged

Conversation

wojtek-t
Copy link
Member

Ref #26119

This should simplify debugging what requests we are sending to GCE (and why we are throttled in routecontroller).

@zmerlynn @gmarek

@gmarek gmarek added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. labels May 24, 2016
@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 24, 2016
@wojtek-t wojtek-t force-pushed the log_requests_to_gce branch from 5c1433d to 16eec92 Compare May 24, 2016 15:20
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
@k8s-bot
Copy link

k8s-bot commented May 24, 2016

GCE e2e build/test passed for commit 16eec923c811a81759ababe7bb745093db263e98.

@zmerlynn zmerlynn added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
rl.limiter.Accept()
// TODO: Reduce verbosity once #26119 is fixed.
glog.V(1).Infof("GCE api call: %s %s (throttled for %v)", req.Method, req.URL.String(), time.Now().Sub(startTime))
Copy link
Member

Choose a reason for hiding this comment

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

This looks fine, but do we log at V(1) by default or are you proposing to restart the controller?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that e.g. in GKE by default we don't log on --v=1. So I changed it temporarily to V(0).
I will reduce the verbosity once we understand this.

@wojtek-t wojtek-t force-pushed the log_requests_to_gce branch from 16eec92 to 55fdc1c Compare May 24, 2016 16:14
@wojtek-t
Copy link
Member Author

The only change is V(1) -> V(0), so reapplying lgtm.

@wojtek-t wojtek-t added lgtm "Looks good to me", indicates that a PR is ready to be merged. e2e-not-required and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 24, 2016
@k8s-bot
Copy link

k8s-bot commented May 24, 2016

GCE e2e build/test passed for commit 55fdc1c.

@wojtek-t
Copy link
Member Author

This change is pretty trivial - merging manually.

@wojtek-t wojtek-t merged commit 7ad337a into kubernetes:master May 24, 2016
k8s-github-robot pushed a commit that referenced this pull request May 26, 2016
Automatic merge from submit-queue

GCE provider: Revert rate limits

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]() This reverts #26140 and #26170. After testing with #26263, #26140 is unnecessary, and we need to be able to prioritize normal GET / POST requests over operation polling requests, which is what the pre-#26140 requests do.

c.f. #26119
@wojtek-t wojtek-t deleted the log_requests_to_gce branch June 15, 2016 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants