Add dbname to mango exec stats

[chewbranca]

Overview

This PR includes the database name in the "mango-stats" alongside the existing data around documents processed to generate the request. The motivation here is two fold: 1) there's a scenario where chttpd:maybe_log returns false and the chttpd socket handling the request has died where a front load balancer processing the request never got the X-Couch-Request-ID, thereby losing the connection of the underlying request that induced the particular load; this is especially problematic when the socket died because the request was excessively long. 2) this allows for grouping by dbname directly on the reports, making it easier to quickly scan and summarize activity from the report.

This PR also sneaks in a patch to mango_cursor_nouveau to include the recent stats logic patch.

Testing recommendations

I've added a new test to show the dbname field arrives in the couch_log:report logic yet does not extend the field into the http response when the _find query does execution_stats=true.

Checklist

• Code is written and works correctly
• Changes are covered by tests
• Any new configurable parameters are documented in rel/overlay/etc/default.ini
• Documentation changes were made in the src/docs folder
• Documentation changes were backported (separated PR) to affected branches

[pgj]

Or you could just simply say:

Base = [{dbname, Stats#execution_stats.dbname} || IncludeDbName]

[chewbranca]

I'm not quite following your suggestion, we need to include the tuple {dbname, Stats#execution_stats.dbname} in the list conditionally. I think perhaps you're suggesting to move the construction of the the base into the arity one function head, like:

to_json(Stats) ->
    to_json(Stats, [{dbname, Stats#execution_stats.dbname}]).

to_json(Stats, Base) ->
  {[
    ...
    | Base
  ]}.

But that would change the caller here https://github.com/apache/couchdb/pull/4990/files/69f80aa3a270030601ebd111bd3a1dc62a5df8fa#diff-478704bf59b4beb680fc69863dfe63a544cfea5d6dca7507d6ce080c84b6cb35R119 to have to supply the base list into to_json, eg to_json(Stats, []) instead of to_json(Stats, false), and I don't think having to supply the base list instead of a boolean is a cleaner approach.

Is this what you were suggesting or something else? I agree that it's a bit clunky for sure.

[pgj]

There is no need to change the function's interface. What I suggested is a semantical equivalent of your original definition. This alternative syntax is based on how list comprehensions in Erlang work. The Boolean value on the right-hand side of the || symbol acts as a predicate and the value on the left-hand side is added to the resulting list if that is true otherwise omitted.

For example, consider the following in the Erlang shell:

> [1 || true].
[1]
> [1 || false].
[]

List comprehensions do not require the use of variables, hence this formulation becomes a valid although unusual instance.

[chewbranca]

Ahhhhh I see what you mean, I haven't seen that structure before. Interesting, but seems a bit esoteric. Perhaps if we make that a common pattern in the codebase.

[pgj]

When applied in a sequence, it works out quite nicely:

couchdb/src/mango/src/mango_idx_view.erl

Lines 137 to 140 in 16bf9df

	Reason =
	[needs_text_search || not NotTextSearch] ++
	[field_mismatch || not SelectorHasRequiredFields] ++
	[sort_order_mismatch || not CanUseSort],

[pgj]

That said, it would look nice in this case as well. Of course, this is just a suggestion, I am adding this example for consideration only where Base is removed completely:

   {[
        {total_keys_examined, Stats#execution_stats.totalKeysExamined},
        {total_docs_examined, Stats#execution_stats.totalDocsExamined},
        {total_quorum_docs_examined, Stats#execution_stats.totalQuorumDocsExamined},
        {results_returned, Stats#execution_stats.resultsReturned},
        {execution_time_ms, Stats#execution_stats.executionTimeMs}
        | [{dbname, Stats#execution_stats.dbname} || IncludeDbName]
    ]}.

[pgj]

Nice catch!

[pgj]

+1 if passes the CI

[pgj]

The Mango integration test suites (make mango-test) are failing, most of them are HTTP 500s:

[2024-03-05T14:04:18.471Z] Ran 384 tests in 12.251s
[2024-03-05T14:04:18.471Z] 
[2024-03-05T14:04:18.471Z] FAILED (failures=3, errors=75, skipped=143)

[pgj]

This seems to be the cause:

[error] 2024-03-05T14:20:35.042023Z [email protected] <0.757.0> ca0f3d4937 req_err(3855649447) badrecord : undefined
    [<<"mango_execution_stats:log_start/1 L106">>,<<"mango_cursor_view:execute/3 L220">>,<<"mango_httpd:handle_find_req/2 L213">>,<<"mango_httpd:handle_req/2 L35">>,<<"chttpd:handle_req_after_auth/2 L416">>,<<"chttpd:process_request/1 L394">>,<<"chttpd:handle_request_int/1 L329">>,<<"mochiweb_http:headers/6 L140">>]

[pgj]

Okay, cool. Looks like all the issues have been fixed.