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

Add latency stats items to S3 API and velvet calls [JIRA: RCS-220] #1180

Merged
merged 9 commits into from
Jul 10, 2015

Conversation

shino
Copy link
Contributor

@shino shino commented Jul 9, 2015

Part 2 of stats improvement following #1165 (RCS-204) and address #961 (RCS-37) (RCS-217) partially.

This PR adds:

  • latency/counter stats for S3 API callback executions and
  • latency/counter stats for velvet calls.

@shino shino added this to the 2.1.0 milestone Jul 9, 2015
- Change command to switch stanchion to riak-cs-admin, avoiding
  depricated warning
- Fix stats item number
- Fix stats item name for back pressure sleep
@shino
Copy link
Contributor Author

shino commented Jul 9, 2015

Fixed riak_test failures, became ready for review actually.

Action = case Operation of
create -> created;
delete -> deleted;
_ -> undefined
end,
Copy link
Contributor

Choose a reason for hiding this comment

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

This indentation looks strange to me, which should be aligned with case.

@shino
Copy link
Contributor Author

shino commented Jul 9, 2015

Thanks for pointing out. Fixed as to two comments above and pushed.

@Basho-JIRA Basho-JIRA changed the title Add latency stats items to S3 API and velvet calls Add latency stats items to S3 API and velvet calls [JIRA: RCS-220] Jul 10, 2015
ok = riak_cs_stats:update_with_start(StatName,
StartTime),
X
riak_cs_user:save_user(UpdUser, UserObj, RcPid)
Copy link
Contributor

Choose a reason for hiding this comment

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

So are we supposed to measure this call in next pull request?

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. Stats in riakc calls are NOT included. Will be in next.

@@ -157,6 +162,7 @@ delete_resource(RD, Ctx=#context{user=User,
ok ->
riak_cs_dtrace:dt_bucket_return(?MODULE, <<"bucket_delete">>,
[200], [riak_cs_wm_utils:extract_name(User), Bucket]),
ok = riak_cs_stats:update_with_start([bucket, delete], StartTime),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you have update_with_start/3 here? Isn't this caught at riak_cs_wm_common:update_stats ?

@kuenishi
Copy link
Contributor

How about adding items in riak_cs_stats:duration_metrics/0 to stats_test.erl? Maybe it's also okay to defer to test sprint. Anyway all work is great. I am tempted to +1.

@shino
Copy link
Contributor Author

shino commented Jul 10, 2015

Thanks for review. Pushed the fix for your comments. Please let me defer modification of stats_test 😅

@kuenishi
Copy link
Contributor

stats_test is now failing, but let's fix it next time.

borshop added a commit that referenced this pull request Jul 10, 2015
Add latency stats items to S3 API and velvet calls  [JIRA: RCS-220]

Reviewed-by: kuenishi
@kuenishi
Copy link
Contributor

nooooooooooooooooooo

@kuenishi
Copy link
Contributor

The fail was about eunit. All tests have succeeded but failed after that.

  All 202 tests passed.

@kuenishi
Copy link
Contributor

@borshop: retry

borshop added a commit that referenced this pull request Jul 10, 2015
Add latency stats items to S3 API and velvet calls  [JIRA: RCS-220]

Reviewed-by: kuenishi
@kuenishi
Copy link
Contributor

@borshop merge

@borshop borshop merged commit 66f4012 into develop Jul 10, 2015
@kuenishi kuenishi deleted the feature/stats-at-wm-common branch July 10, 2015 16:38
@Basho-JIRA
Copy link

TODO: write down release note

_[posted via JIRA by Kota Uenishi]_

@shino shino mentioned this pull request Jul 17, 2015
23 tasks
@Basho-JIRA
Copy link

This is a part of RCS-11 work; release notes will be written there.

_[posted via JIRA by Kota Uenishi]_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants