pgj commented on code in PR #4990:
URL: https://github.com/apache/couchdb/pull/4990#discussion_r1503535168
##########
src/mango/src/mango_execution_stats.erl:
##########
@@ -37,12 +37,23 @@
-define(SHARD_STATS_KEY, mango_shard_execution_stats).
to_json(Stats) ->
+ to_json(Stats, true).
+
+to_json(Stats, IncludeDbName) ->
+ Base =
Review Comment:
Or you could just simply say:
```erlang
Base = [{dbname, Stats#execution_stats.dbname} || IncludeDbName]
```
##########
src/mango/src/mango_execution_stats.hrl:
##########
@@ -16,5 +16,6 @@
totalQuorumDocsExamined = 0,
resultsReturned = 0,
executionStartTime,
- executionTimeMs
+ executionTimeMs,
+ dbname
Review Comment:
Since all the other fields have full-length names, `databaseName` might be a
better fit here for consistency.
##########
src/mango/src/mango_cursor_text.erl:
##########
@@ -109,7 +110,7 @@ execute(Cursor, UserFun, UserAcc) ->
user_fun = UserFun,
user_acc = UserAcc,
fields = Cursor#cursor.fields,
- execution_stats = mango_execution_stats:log_start(Stats),
+ execution_stats = mango_execution_stats:log_start(Stats, DbName),
Review Comment:
I think the `create` family of functions might be a better place to
initialize the `#execution_stats{}` record with the database name immediately.
So that you would not need add it in `execute/3` and
`mango_execution_stats:log_start/1` would not need an extra parameter for that.
For example, in case of `mango_cursor_text`, something along these lines:
```diff
diff --git a/src/mango/src/mango_cursor_text.erl
b/src/mango/src/mango_cursor_text.erl
index ee1d962f7..5cc06344f 100644
--- a/src/mango/src/mango_cursor_text.erl
+++ b/src/mango/src/mango_cursor_text.erl
@@ -50,7 +50,8 @@ create(Db, {Indexes, Trace}, Selector, Opts0) ->
?MANGO_ERROR(multiple_text_indexes)
end,
- Opts = unpack_bookmark(couch_db:name(Db), Opts0),
+ DbName = couch_db:name(Db),
+ Opts = unpack_bookmark(DbName, Opts0),
DreyfusLimit = get_dreyfus_limit(),
Limit = erlang:min(DreyfusLimit, couch_util:get_value(limit, Opts,
mango_opts:default_limit())),
@@ -66,7 +67,8 @@ create(Db, {Indexes, Trace}, Selector, Opts0) ->
opts = Opts,
limit = Limit,
skip = Skip,
- fields = Fields
+ fields = Fields,
+ execution_stats = #execution_stats{dbname = DbName}
}}.
explain(Cursor) ->
```
##########
src/mango/src/mango_cursor_nouveau.erl:
##########
@@ -128,6 +129,8 @@ execute(Cursor, UserFun, UserAcc) ->
{FinalUserAcc0, Stats1} = mango_execution_stats:maybe_add_stats(
Opts, UserFun, Stats0, FinalUserAcc
),
+ %% This needs Stats1 as log_end is called in maybe_add_stats
+ mango_execution_stats:log_stats(Stats1),
Review Comment:
Nice catch!
--
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]
