iilyak commented on code in PR #5666:
URL: https://github.com/apache/couchdb/pull/5666#discussion_r2373230646
##########
src/couch/src/couch_file.erl:
##########
@@ -621,7 +621,7 @@ handle_info({'DOWN', Ref, process, _Pid, _Info},
#file{db_monitor = Ref} = File)
false -> {noreply, File}
end.
-format_status(_Opt, [PDict, #file{} = File]) ->
+format_status([PDict, #file{} = File]) ->
Review Comment:
Interesting that OTP has an undocumented callback
[`Mod:system_get_state/1`](https://github.com/erlang/otp/blame/6d3202747f78850325fa5fd968e5dd661a18932b/lib/stdlib/src/sys.erl#L960).
> But why put it in pdict, and then call sys:get_status(Pid) and fetch it
from pdict when the default implementation of sys:get_status(Pid) already
returns the state which has the filepath.
The filepath field is a [recent
addition](https://github.com/apache/couchdb/commit/c1a539b9c6a172d7bd89c7eddbbf129017549a84#diff-6737c3d19b856f05add13070142596c38642b1ecb075f139e82520b391e5eb30R40).
Previously the filepath was not available in the state record. So it was
artificially added for `sys:get_state/1` and termination use cases.
I think we can rewrite the format_status to use the `filepath` field. What
we wanted to make sure is that we pass it correctly via
[`gen_server:error/7`](https://github.com/erlang/otp/blob/master/lib/stdlib/src/gen_server.erl#L2768),
through formatters in `sys` into the
[`couch_log_formatter:do_format/3`](https://github.com/apache/couchdb/blob/main/src/couch_log/src/couch_log_formatter.erl#L89).
The details are escaping me at the moment. It could be that with the
addition of a `filepath` into the state record we no longer need custom
format_status. Someone would need to test it to make sure the path is
propagated to the logs on termination.
--
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]
