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

Feature/stats2 #99

Merged
merged 6 commits into from Jul 24, 2015
Merged

Feature/stats2 #99

merged 6 commits into from Jul 24, 2015

Conversation

kuenishi
Copy link
Contributor

  • Add Riak access stats, mochiweb stats and system stats
  • Some code cleanup

@kuenishi kuenishi added this to the 2.1.0 milestone Jul 22, 2015
@@ -184,11 +184,17 @@

check_no_multipart_uploads(Bucket, RiakPid) ->
HashBucket = stanchion_utils:to_bucket_name(objects, Bucket),
{ok, Keys} = riakc_pb_socket:list_keys(RiakPid, HashBucket),


Copy link
Contributor

Choose a reason for hiding this comment

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

Two blank lines.

@shino
Copy link
Contributor

shino commented Jul 23, 2015

Will some tests come in the next round?

@kuenishi
Copy link
Contributor Author

Those tests will be in riak_cs repo as integration test. Fair?

@shino
Copy link
Contributor

shino commented Jul 23, 2015

Yup. Just want there be an issue or jira ticket for that (to cover my poor memory).

@@ -1,346 +0,0 @@
%% ---------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Great eliminator 🔥

@shino
Copy link
Contributor

shino commented Jul 23, 2015

The module stanchion_wm_buckets is used from velvet but not used from riak cs.
There is another PR for riak cs to merge velvet, so it seems good time to
delete the API completely.

Suggestion:

  • Remove stanchion_wm_buckets.erl
  • Then stanchion_utils:get_buckets() will be unused,
    get_value() in stanchion_utils will be unused.
    At last, [riakc, get_manifest] stats key can be removed in get_value function
    and stats definition in stanchion_stats.
  • Remove velvet:list_buckets() in velvet merge PR to cs

@shino
Copy link
Contributor

shino commented Jul 23, 2015

There was one wrong point in the above comment. [riakc, get_manifest] is used in other parts, can not be removed.

@@ -771,13 +717,18 @@ get_keys_and_values(BucketName) ->
{ok, riakc_obj:riakc_obj()} | {error, notfound}.
get_manifests_raw(RiakcPid, Bucket, Key) ->
ManifestBucket = to_bucket_name(objects, Bucket),
riakc_pb_socket:get(RiakcPid, ManifestBucket, Key).
{Res, TAT} = ?TURNAROUND_TIME(riakc_pb_socket:get(RiakcPid, ManifestBucket, Key)),
stanchion_stats:update([riakc, get_manifests], TAT),
Copy link
Contributor

Choose a reason for hiding this comment

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

Existing r_t caches typo 😹

Error log was as follows:

2015-07-23 11:41:42.343 [debug] <0.224.0>@stanchion_wm_bucket:delete_resource:123 Bucket: <<"riak-test-bucket">> Requester: <<"FSUNXVTYOMMTSX3Z_XJX">>
2015-07-23 11:41:42.345 [debug] <0.207.0>@stanchion_stats:update:95 Updating [riakc,get_cs_bucket] (1139)
2015-07-23 11:41:42.349 [debug] <0.207.0>@stanchion_stats:update:95 Updating [riakc,list_all_manifest_keys] (4026)
2015-07-23 11:41:42.350 [debug] <0.207.0>@stanchion_stats:update:95 Updating [riakc,get_manifests] (1005)
2015-07-23 11:41:42.350 [error] <0.207.0>@stanchion_utils:do_bucket_op:558 Error on updating bucket riak-test-bucket: {error,{badmatch,{error,not_found}}}
2015-07-23 11:41:42.350 [debug] <0.224.0>@stanchion_server:delete_bucket:105 [bucket,delete] {error,{error,{badmatch,{error,not_found}}}} 7580
2015-07-23 11:41:42.351 [debug] <0.224.0>@stanchion_stats:update:95 Updating [bucket,delete] (7580)
2015-07-23 11:41:42.358 [error] <0.224.0> Webmachine error at path "/buckets/riak-test-bucket" : {error,function_clause,[{stanchion_response,status_code,[{error,{badmatch,{error,not_found}}}],[{file,"src/stanchion_response.erl"},{line,65}]},{stanchion_response,api_error,3,[{file,"src/stanchion_response.erl"},{line,85}]},{webmachine_resource,resource_call,3,[{file,"src/webmachine_resource.erl"},{line,186}]},{webmachine_resource,do,3,[{file,"src/webmachine_resource.erl"},{line,142}]},{webmachine_decision_core,resource_call,1,[{file,"src/webmachine_decision_core.erl"},{line,48}]},{webmachine_decision_core,...},...]}

@shino
Copy link
Contributor

shino commented Jul 23, 2015

Finished first round of review. Overall diff seems nice, bunch of code deletion is great 🏆
Will run r_t after comments are addressed.

@kuenishi
Copy link
Contributor Author

Updated. Will run r_t with cs#1197, too.

@kuenishi
Copy link
Contributor Author

Woops, updated

@kuenishi
Copy link
Contributor Author

Fmm, many riak_test are failing with 500 ... cannot be merged yet.

@kuenishi
Copy link
Contributor Author

Yay!

0 Tests Failed
34 Tests Passed
That's 100.0% for those keeping score

real    101m38.407s
user    15m54.576s
sys     3m42.104s

@shino
Copy link
Contributor

shino commented Jul 24, 2015

💪

shino added a commit that referenced this pull request Jul 24, 2015
@shino shino merged commit b61d219 into develop Jul 24, 2015
@shino shino deleted the feature/stats2 branch July 24, 2015 03:16
@shino shino mentioned this pull request Jul 24, 2015
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.

None yet

2 participants