You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by brkolla <gi...@git.apache.org> on 2016/05/03 20:50:02 UTC
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
GitHub user brkolla opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/118
64560 add metrics bulk docs count
Add a new counter "bulk_request_docs" to track the number of documents being handled as part of the bulk request.
FogBugz 64560.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-chttpd 64560-add-metrics-bulk-docs-count
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/118.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #118
----
commit 374476bdb609b1fc93a6ed6b8f5ab861fe60f266
Author: brkolla <bk...@cloudant.com>
Date: 2016-04-21T13:27:11Z
Add new counter to track the number of docs being processed using
bulk_docs request.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by brkolla <gi...@git.apache.org>.
Github user brkolla commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216974477
I think I have to revisit this and come up with a new metric to track the bulk gets and probably use one metric that tracks all the request that would result in bulk gets (_all_docs etc..) . We are still discussing on how we can do that.
I guess I will have to close this PR as we are not going to go ahead with this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Add new metric (histogram) to track n...
Posted by brkolla <gi...@git.apache.org>.
Github user brkolla commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-217954638
@chewbranca I have updated the code to add a new metric bulk_reads to track the number of doc reads done as part of the bulk_get. Can you review this code?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by brkolla <gi...@git.apache.org>.
Github user brkolla commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216923999
I am actually asking Aaron about this in Slack. To be honest, I don't fully understand the difference between the histogram and the regular counter and if its possible to have one metric that does the both. Will ask around and find out more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Add new metric (histogram) to track n...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#discussion_r62553105
--- Diff: src/chttpd_db.erl ---
@@ -448,6 +448,7 @@ db_req(#httpd{method='POST', path_parts=[_, <<"_bulk_get">>]}=Req, Db) ->
undefined ->
throw({bad_request, <<"Missing JSON list of 'docs'.">>});
Docs ->
+ couch_stats:update_histogram([couchdb, httpd, bulk_reads], length(Docs)),
--- End diff --
I think the name is confusing. If we want to count only "/_bulk_get" requests here, better give such name to the metric instead of generic bulk_reads. Otherwise, if you want to use "bulk_reads", should we count requests with include_docs=true to views and all_docs as bulk reads?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216970841
Yeah I agree with @kxepal on this, using a histogram is the correct choice here. It looks like there was also some confusion on this issue as @rnewson snuck in the actual fix for this issue back in https://github.com/cloudant/couchdb-chttpd/commit/a49c47417cc9190f8b5afe0c058c9a742a6cf587.
I do think it's worthwhile to also track the doc count in `_bulk_get` and that should be switched to a histogram like the above commit. The big question there is whether to use a different histogram to be able to distinguish between _bulk_get reads and _bulk_docs writes. I think having them separate would be reasonable.
Also, we should get similar stats for quantify of docs returned and provided in _all_docs and views and what not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216936156
@brkolla Histogram uses sliding time window and provides percentile-wise information about the recorded numbers. Just compare output of both these metrics to see the difference.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Add new metric (histogram) to track n...
Posted by brkolla <gi...@git.apache.org>.
Github user brkolla commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-218225070
@kxepal I am not really sure, but this seems to be only used during the replication. I guess, we could change the name to bulk_get_docs? @chewbranca?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by brkolla <gi...@git.apache.org>.
Github user brkolla commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216881959
@kxepal ? I have asked the same question in 64560 and discussed it with Aaron. As per my understanding, counter gives a different perspective and probably easy to identify the spikes and trends? Can you add a comment to Aaron/Russel/Ops in 64560 and get his perspective on why this metric would help Operations?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Add new metric (histogram) to track n...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-218229516
@brkolla Not only. Replication is just a client for CouchDB. Other users may be this endpoint in own way (and for profit!).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216914075
@brkolla Sorry, I can't since I'm not from Cloudant team. Can you ping them here or share the reason why? because I'm -1 on having two metrics that does the same thing while one of them (histogram) is actually what is really useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216737066
Why? When you have a line above: `couch_stats:update_histogram([couchdb, httpd, bulk_docs], length(DocsArray))` which is more informative than just constantly incremented counter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216972329
Oh, it also counts `_bulk_get`. I miss that moment, sorry. But that should be a different metric and should have a correct name anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: 64560 add metrics bulk docs count
Posted by chewbranca <gi...@git.apache.org>.
Github user chewbranca commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/118#issuecomment-216974481
Yeah agreed, _bulk_get is sufficiently different we should track it separately.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---