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.

Sign up for GitHub

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

Jump to bottom

update perf run to the new mixer config #740

Merged
merged 6 commits into from
Sep 25, 2017
Merged

update perf run to the new mixer config #740

merged 6 commits into from
Sep 25, 2017

Conversation

ldemailly
Copy link
Contributor

@ldemailly ldemailly commented Sep 11, 2017 •
edited
Loading 

NONE

@ldemailly
update perf run to the new mixer config
ce97517 
ps: currently also needs --useAst to work

```release-note
NONE
```
@douglas-reid
Copy link
Contributor

are we postponing merging of this until --useAst is not required?

@ldemailly
Copy link
Contributor Author

No I want to merge without waiting

@ldemailly
Copy link
Contributor Author

/retest

@istio-testing
Copy link
Collaborator

istio-testing commented Sep 13, 2017 •
edited
Loading

@ldemailly: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/new-e2e-rbac_no_auth.sh cf27e44 link /test new-e2e-rbac_no_auth

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

kyessenov
kyessenov reviewed Sep 13, 2017
View reviewed changes
devel/setup_run
cd mixer; set +x; source bin/use_bazel_go.sh ; set -x; cd ..
./istio/bazel-bin/devel/fortio/cmd/fortio/fortio server &
( cd proxy/src/envoy/mixer; ./start_envoy > /tmp/envoy.log ) &
# add -v=5 for verbose/debug
./mixer/bazel-bin/cmd/server/mixs server --configStoreURL=fs://$(pwd)/mixer/testdata/configroot --logtostderr 2> /tmp/mixs.2.log &
./mixer/bazel-bin/cmd/server/mixs server --configStore2URL=fs://$(pwd)/mixerconfig --configStoreURL=fs://$(pwd)/emptydir --logtostderr 2> /tmp/mixs.2.log &
Copy link
Contributor

Choose a reason for hiding this comment

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

is default level 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes afaik, those logs aren't too floody (modulo logging bugs that keep creeping up)

@istio-merge-robot
Copy link

@ldemailly PR needs rebase

@istio-merge-robot istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 13, 2017
@douglas-reid
Copy link
Contributor

/lgtm

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: douglas-reid

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @douglas-reid @ldemailly

@istio-merge-robot istio-merge-robot removed lgtm needs-rebase Indicates a PR needs to be rebased before being merged labels Sep 23, 2017
@ldemailly
Copy link
Contributor Author

rerunning now to see if it still works

@ldemailly
Copy link
Contributor Author

getting

[libprotobuf ERROR external/mixerclient_git/src/quota_cache.cc:116] Quota response did not have quota for: RequestCount

with this setup

@ldemailly
Copy link
Contributor Author

@ZackButcher / @rshriram
this PR isn't related to main line (so no risk) but is needed to run the benchmark - please approve

ZackButcher
ZackButcher approved these changes Sep 25, 2017
View reviewed changes
Copy link
Contributor

@ZackButcher ZackButcher left a comment

Choose a reason for hiding this comment

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

LGTM

@ldemailly ldemailly merged commit 2eb390a into master Sep 25, 2017
@ldemailly ldemailly deleted the ldemailly-perf-update branch September 25, 2017 23:04
rshriram pushed a commit that referenced this pull request Oct 30, 2017
@ldemailly
update perf run to the new mixer config (#740)
0cbe21a 
ps: currently also needs --useAst to work

```release-note
NONE
```
Former-commit-id: 2eb390a
vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017
@ldemailly
update perf run to the new mixer config (istio#740)
c88cd06 
ps: currently also needs --useAst to work

```release-note
NONE
```
Former-commit-id: 2eb390a
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
@ldemailly
update perf run to the new mixer config (#740)
5d78a0e 
ps: currently also needs --useAst to work

```release-note
NONE
```
Former-commit-id: 2eb390a
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
@istio-testing
add serviceAnnotation to KubernetesResourcesSpec. (istio#740)
dc2f1b3
0x01001011 pushed a commit to thedemodrive/istio that referenced this pull request Jul 16, 2020
@geeknoid
Fix a comment. (istio#740)
6c1f299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyessenov kyessenov kyessenov left review comments

@costinm costinm costinm approved these changes

@ZackButcher ZackButcher ZackButcher approved these changes

@mandarjog mandarjog Awaiting requested review from mandarjog

@douglas-reid douglas-reid Awaiting requested review from douglas-reid

Assignees

@douglas-reid douglas-reid

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@ldemailly @douglas-reid @istio-testing @istio-merge-robot @costinm @ZackButcher @kyessenov @googlebot