chewbranca commented on code in PR #5602:
URL: https://github.com/apache/couchdb/pull/5602#discussion_r2221034383
##########
rel/overlay/etc/default.ini:
##########
@@ -1150,3 +1150,63 @@ url = {{nouveau_url}}
;mem3_shards = true
;nouveau_index_manager = true
;dreyfus_index_manager = true
+
+; Couch Stats Resource Tracker (CSRT)
+[csrt]
+enable = true
+enable_init_p = true
+enable_reporting = true
+;enable = false
+;enable_init_p = false
+;enable_reporting = false
+;enable_rpc_reporting = false
+
+; Truncate reports to not include zero values for counter fields. This is a
+; simple way to save space and should be left enabled unless you need a fixed
+; output structure in the process lifecycle reports.
+;should_truncate_reports = true
+
+; Limit queries to a maxinum number of rows
+;query_limit = 10000
+
+; CSRT Rexi Server RPC Worker Spawn Tracking
+; Enable these to enable additional metrics for RPC worker spawn rates
+; measuring how often RPC workers are spawned by way of rexi_server:init_p.
+; Mod and Function are separated by double underscores.
+[csrt.init_p]
+fabric_rpc__all_docs = true
+fabric_rpc__changes = true
+fabric_rpc__get_all_security = true
+fabric_rpc__map_view = true
+fabric_rpc__open_doc = true
+fabric_rpc__open_shard = true
+fabric_rpc__reduce_view = true
+fabric_rpc__update_docs = true
Review Comment:
These `[csrt.init_p]` values are awkward... and unfortunately _do_ need to
be set here by default. All of the other values in this `default.ini` have
default values to be set to and disabled, but these need to be set to true to
have a lookup enabled for these settings and others.
The original idea with `[csrt.init_p]` is that this would prevent the
counter increments from triggering ever to prevent the tracking space for those
counters to be enabled, but the folsom should_notify stuff isn't used directly
anymore and besides the metrics are being picked up from the declaration of
`stats_descriptions.cfg`.
We need a positive lookup table for these values to return true to indicate
they are they desired subset of operations spawned in `rexi_server:init_p/3`,
as otherwise we'd create metrics for _all_ `fabric_rpc:*` functions.
Alternatively, this could be a static map in the `csrt.hrl` file, but we need a
way to add more to these later. I'm admittedly not a huge fan of this
particular aspect of the init_p logic, but like I said, we need a positive
lookup table somewhere.
As for the double underscore, we've talked about this in the last PR,
there's _zero_ precedent for the use of `:` in the codebase, and the only
keyword that isn't `[a-z0-9_]` is `||*` in `partitioned||*`, eg:
```
(! 11775)-> grep '^[a-z;].* = ' rel/overlay/etc/default.ini | sed
's/^\([a-z;][^ ]*\) =.*$/\1/' | grep -v ' ' | grep '[^a-z0-9_;]'
partitioned||*
```
and I was not able to find documentation that `:` is valid for default.ini,
so I did not use that. Because of the `||` use, I used it in IOQ in the past
https://github.com/apache/couchdb-ioq/blob/main/include/ioq.hrl#L26 but found
`||` to be ugly, so I went with double underscore.
Anyways... just let's just remove this entirely, the dynamic config toggle
approach with the specific metrics didn't workout as I had wanted, so let's
just go back to hardcoding these directly, that'll simplify this, eliminate the
ini section entirely, and also get rid of the ugly string conversion in
`csrt_util:fabric_conf_key/1`.
Here's the patch to hardcode it, and I'll drop out the other components in a
bit:
```
(! 11789)-> git diff src/couch_stats/src/csrt.erl
diff --git a/src/couch_stats/src/csrt.erl b/src/couch_stats/src/csrt.erl
index 6c098bfea..3532a3b2b 100644
--- a/src/couch_stats/src/csrt.erl
+++ b/src/couch_stats/src/csrt.erl
@@ -431,8 +431,25 @@ maybe_inc(Stat, Val) ->
end.
-spec should_track_init_p(Stat :: [atom()]) -> boolean().
-should_track_init_p([Mod, Func, spawned]) ->
- is_enabled_init_p() andalso csrt_util:should_track_init_p(Mod, Func);
+%% "Example to extend CSRT"
+%% should_track_init_p([fabric_rpc, foo, spawned]) ->
+%% is_enabled_init_p();
+should_track_init_p([fabric_rpc, all_docs, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, changes, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, get_all_security, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, map_view, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, open_doc, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, open_shard, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, reduce_view, spawned]) ->
+ is_enabled_init_p();
+should_track_init_p([fabric_rpc, update_docs, spawned]) ->
+ is_enabled_init_p();
should_track_init_p(_Metric) ->
false.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
