You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/05/20 21:22:55 UTC
[GitHub] [couchdb] noahshaw11 opened a new pull request, #4033: Implement view_report function
noahshaw11 opened a new pull request, #4033:
URL: https://github.com/apache/couchdb/pull/4033
## Overview
Implement `view_report` function to facilitate memory leak detections.
## Testing recommendations
## Related Issues or Pull Requests
## Checklist
- [ ] Code is written and works correctly
- [ ] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916861685
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +157,139 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
Review Comment:
How about adding all fields from the `Info`? They all seem to be useful for analyzing view performance.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ...,
reduce_funs => ...,
def => ...,
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(State, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## View {name}
couch_debug:print_report(PartivularViewInfo)
### map
{def}
### reduce
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
map_name => ..., <<----- from map_names
def => ..., <<----- move it here from views if it is common
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ..., <--- // this field is not needed
reduce_funs => ..., <--- [{Name, Def}] // this field is not needed
def => ..., <--- A def from Def of reduce_funs
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(State, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## Index {idx_name} <---- taken from #mrst.idx_name
couch_debug:print_report(ParticularViewInfo)
### map {map_name} <--- taken from #mrview.map_names
{def}
### reduce {reduce_name} <- taken from #mrview.reduce_funs == ViewName
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916846716
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,100 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map that includes the signature, db_name, idx_name, update_seq,
+ purge_seq, view_file_path, and pending_updates for an index.
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the id_num, update_seq, purge_seq, reduce_funs, def,
+ btree_size, and options for a ViewName if specified or all views if not.
+
+ ---
+ ", []);
+help(index_view_state) ->
+ io:format("
+ index_view_state(Pid)
+ index_view_state(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the index state returned by index_state/1 and the view
+ state returned by view_state/2. Like view_state/2, a ViewName can be specified or not.
+
+ ---
+ ", []);
+help(index_report) ->
+ io:format("
+ index_report(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Prints a report for the index state of an mrview index. The report includes signature, db_name,
+ idx_name, update_seq, purge_seq, view_file_path, and pending_updates.
+
+ ---
+ ", []);
+help(view_report) ->
+ io:format("
+ view_report(PidOrIdxState)
+ view_report(Pid, ViewName)
+ view_report(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index.
Review Comment:
IdxState: State of an mrview index (`#mrst{}` record).
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916873515
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,100 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map that includes the signature, db_name, idx_name, update_seq,
+ purge_seq, view_file_path, and pending_updates for an index.
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the id_num, update_seq, purge_seq, reduce_funs, def,
+ btree_size, and options for a ViewName if specified or all views if not.
+
+ ---
+ ", []);
+help(index_view_state) ->
+ io:format("
+ index_view_state(Pid)
+ index_view_state(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the index state returned by index_state/1 and the view
+ state returned by view_state/2. Like view_state/2, a ViewName can be specified or not.
+
+ ---
+ ", []);
+help(index_report) ->
Review Comment:
For all report functions please provide an example of the output (see [example in `couch_index_debug:print_linked_processes/0`](https://github.com/apache/couchdb/blob/main/src/couch_index/src/couch_index_debug.erl#L60:L73).
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916796523
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
I've noticed that the table specifically for `index_report`/`index_view_report`/`view_report` is too wide. So I was looking for opportunities to improve that. My first idea was to update TableSpec format to support `auto` in the column with. Turned out it is a bit complicated, and requiring a change in already implemented helper functions. So I decided it is not worth doing.
My second observation was the fact that for the above mentioned functions the first column is a set of field names. We know the field names upfront. They are:
- `id_num`
- `update_seq`
- `purge_seq`
- `reduce_funs`
- `def`
- `btree_size`
- `options`
- `signature`
The longest field name is 11 characters. So we could replace 50 with 11 (or 13). In order to do it we need a version of this function with arity 2.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#issuecomment-1146090017
I also want to clarify the need for a `print_report` function on top of the existing `print_table` function. If the `print_table` function is used, the following is printed:
```
> couch_mrview_debug:view_report().
| info | signature
| signature | 39b98489b79419503499bbee402040c7
| info | db_name
| db_name | shards/00000000-ffffffff/dbv1.1654270700
| info | idx_name
| idx_name | _design/dbv1ddoc
| info | reduce_funs
| reduce_funs |
| info | def
| def | function(doc){emit(doc.id, 1);}
| info | update_seq
| update_seq | 3
| info | purge_seq
| purge_seq | 0
| info | view_file_path
| view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1654270700_design/mrview/39b98489b79419503499bbee402040c7.view
| info | pending_updates
| pending_updates | 0
[ok,ok,ok,ok,ok,ok,ok,ok,ok]
```
On the other hand, if `print_report` is used, the following is printed:
```
> couch_mrview_debug:view_report().
| info | value
| signature | 39b98489b79419503499bbee402040c7
| db_name | shards/00000000-ffffffff/dbv1.1654270218
| idx_name | _design/dbv1ddoc
| reduce_funs |
| def | function(doc){emit(doc.id, 1);}
| update_seq | 3
| purge_seq | 0
| view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1654270218_design/mrview/39b98489b79419503499bbee402040c7.view
| pending_updates | 0
[ok,ok,ok,ok,ok,ok,ok,ok,ok]
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882712227
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
+ Report = [
+ {signature, [{signature, couch_index_util:hexsig(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, element(6, IdxState)}]},
+ {reduce_funs, [{reduce_funs, element(1, element(10, IdxState))}]},
+ {def, [{def, element(7, lists:nth(1, element(11, IdxState)))}]},
Review Comment:
`lists:nth(1` -> `[Hd | _] = Something,`
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891701659
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
Yes, I made the change. It will actually have to have to be placed into a list too.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891665942
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
The `table_row` function expects the data to be structured that way from what I could see.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#issuecomment-1133411418
Example return:
```
> couch_debug:view_report().
| info | value
| signature | 3339623938343839623739343139353033343939626265653430323034306337
| db_name | shards/00000000-ffffffff/dbv1.1653068879
| idx_name | _design/dbv1ddoc
| reduce_funs |
| def | function(doc){emit(doc.id, 1);}
| update_seq | 3
| purge_seq | 0
| view_file_path |/dbv1.1653068879_design/mrview/3339623938343839623739343139353033343939626265653430323034306337.view
| pending_updates | 0
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923701451
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
Ah, I understand. I changed it to `print_report_with_info_width/2` to be more specific.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923868527
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +329,41 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, InfoVal} where each InfoKey is unique
+ (unlike print_table/2).
+
+ The output will look similar to:
+
+ |info | value
+ | btree_size | 51
+ | def | function(doc){emit(doc.id, 1);}
Review Comment:
I agree with truncating it, but the question is in which way? How do we distinguish what is important enough to return to the user and what is not?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923877374
##########
src/couch/src/couch_debug.erl:
##########
@@ -289,12 +292,12 @@ help(print_linked_processes) ->
initial Pid to start from. The function doesn't recurse to pids
older than initial one. The output would look like similar to:
```
-couch_debug:print_linked_processes(whereis(couch_index_server)).
-name | reductions | message_queue_len | memory
-couch_index_server[<0.288.0>] | 478240 | 0 | 109696
- couch_index:init/1[<0.3520.22>] | 4899 | 0 | 109456
- couch_file:init/1[<0.886.22>] | 11973 | 0 | 67984
- couch_index:init/1[<0.3520.22>] | 4899 | 0 | 109456
+ couch_debug:print_linked_processes(whereis(couch_index_server)).
Review Comment:
Ah understood. I indented it to stay consistent.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916878556
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
+ TableSpec = [
+ {50, centre, info},
Review Comment:
For consistency with other functions in this module can we use `left` here (for first column)?
- https://github.com/apache/couchdb/blob/main/src/couch/src/couch_debug.erl#L622
- https://github.com/apache/couchdb/blob/main/src/couch/src/couch_debug.erl#L803
Should we update mem_hoggers functions to use `left` as well (in a separate PR)?
I don't have strong opinion on whether we should fix it as well as any preference `left` vs `centre`. I just prefer it to be consistent.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r898350755
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
Review Comment:
Should the following pattern match with `{value, {Key, Value}}` that `couch_util:get_value/3` uses?
```
{value,{mrview,0,3,0,
[<<"dbv1view">>],
[],<<"function(doc){emit(doc.id, 1);}">>,
{btree,<7954.955.0>,
{109,{2,[],24},51},
undefined,undefined,
fun couch_ejson_compare:less_json_ids/2,
#Fun<couch_mrview_util.14.69111627>,snappy},
[]}}
```
I would think not since the arities of the tuples do not match.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r889046119
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
+ Report = [
+ {signature, [{signature, couch_index_util:hexsig(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, element(6, IdxState)}]},
+ {reduce_funs, [{reduce_funs, element(1, element(10, IdxState))}]},
+ {def, [{def, element(7, lists:nth(1, element(11, IdxState)))}]},
Review Comment:
I see, thanks for the core details on the implementation of `lists` in Erlang!
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896023929
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ Report = [
Review Comment:
How about making report a map and making `couch_debug:print_report/1` work with maps or updating `view_report/1` to convert from map to list before calling `print_report`?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested only in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ...,
reduce_funs => ...,
def => ...,
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(PidOrState, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## View {name}
couch_debug:print_report(PartivularViewInfo)
### map
{def}
### reduce
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r912256762
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
Review Comment:
What do you mean by consistent? `view_state/2` returns a map of either
```
#{
ViewName1 => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
},
ViewName2 => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
},
...
}
```
if no `ViewName` is specified or
```
#{
ViewName => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
}
}
```
if a `ViewName` is specified.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923678253
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
I like your approach. However I don't want to take `print_report/2` name. Since in the future I am planing to implement support for reports with more than 2 columns. In this case we would need to pass TableSpec in a second argument. Let's use your approach but name function differently.
```erlang
-export([
...
print_report/1,
print_report_with_id_width/2,
...
]).
print_report(Report) ->
print_report_with_id_width(Report, 50).
print_report_with_id_width(Report, IdWidth) ->
TableSpec = [
{IdWidth, left, info},
{100, right, value}
],
....
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923713227
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +329,41 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, InfoVal} where each InfoKey is unique
+ (unlike print_table/2).
+
+ The output will look similar to:
+
+ |info | value
+ | btree_size | 51
+ | def | function(doc){emit(doc.id, 1);}
Review Comment:
I am curious what we should do for the case when the JavaScript function is longer than table width. Should we truncate the values?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on pull request #4033: Implement index report functions
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#issuecomment-1190435354
> +1, Good work.
Thanks, @iilyak!
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891688479
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
there is a line change in a snippet
```
- io:format("~s~n", [table_row(InfoKey, 2, Value, TableSpec1)])
+ io:format("~s~n", [table_row(InfoKey, 2, {InfoKey, Value}, TableSpec1)])
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911194315
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
+ view_state(IdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
+ end,
+ #{},
+ IdxState#mrst.views
+ ),
+ case ViewName of
+ undefined ->
+ {ok, MrViews};
+ _ ->
+ {ok, #{ViewName => maps:get(ViewName, MrViews)}}
+ end;
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+index_view_state(Pid) when is_pid(Pid) ->
+ index_view_state(Pid, undefined).
+
+index_view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = index_state(Pid),
+ {ok, ViewState} = view_state(Pid, ViewName),
+ {ok, maps:put(views, ViewState, IdxState)}.
+
+index_report(Pid) when is_pid(Pid) ->
+ case index_state(Pid) of
+ {ok, IdxState} ->
+ couch_debug:print_report(maps:to_list(IdxState));
+ Error ->
+ Error
+ end.
+
+view_report(Pid) when is_pid(Pid) ->
Review Comment:
We can remove this clause. Because the first clause of the `view_report/2` would handle this case.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891708408
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +63,27 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_report(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ Report = [
+ {signature, [{signature, couch_util:to_hex(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, IdxState#mrst.idx_name}]},
+ {reduce_funs, [{reduce_funs, MrView#mrview.reduce_funs}]},
+ {def, [{def, MrView#mrview.def}]},
+ {update_seq, [{update_seq, IdxState#mrst.update_seq}]},
+ {purge_seq, [{purge_seq, IdxState#mrst.purge_seq}]},
+ {view_file_path, [{view_file_path, couch_mrview_util:index_file(DbName, Sig)}]},
+ {pending_updates, [lists:keyfind(pending_updates, 1, Info)]}
+ ],
+ couch_debug:print_report(Report);
Review Comment:
Good idea, separated.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r914894858
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => Sig,
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(PidOrIdxState) ->
+ view_state(PidOrIdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ % reduce_funs contains tuples of {Name, ReduceFuns}
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
Review Comment:
Looks like you are just adding a new value. In such case `maps:put/3` would be better.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916854411
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,100 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map that includes the signature, db_name, idx_name, update_seq,
Review Comment:
The inline representation of fields list is hard to read. Can you use something like:
```
For a given index returns a state map that includes following fields:
- db_name
- idx_name
- ...
```
Also update documentation for other functions bellow.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r914976194
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
Review Comment:
Never mind. I think they would need to be written differently for couch_debug and I don't want to complicate this PR.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896028431
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ Report = [
Review Comment:
I prefer having `view_report/1` convert from a map to a list mainly because `print_report` uses `bind_value` which expects a list. Having `view_state/1` return a map is better though.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882711045
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
Review Comment:
Please don't use `element/2`, `the element/2` will stop working when the record definition would change and we don't have compiler errors forcing us to fix this code. Use corresponding records directly.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r914972589
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
Review Comment:
The `first_call/1` and `find_by_first_call` are generic enough and have utility in other debugging activities. Please move them into `couch_debug.erl`. Don't forget to document them. Since they would become public operator facing API.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916878556
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
+ TableSpec = [
+ {50, centre, info},
Review Comment:
For consistency with other functions in this module can we use `left` here?
- https://github.com/apache/couchdb/blob/main/src/couch/src/couch_debug.erl#L622
- https://github.com/apache/couchdb/blob/main/src/couch/src/couch_debug.erl#L803
Should we update mem_hoggers functions to use `left` as well (in a separate PR)?
I don't have strong opinion on whether we should fix it as well as any preference `left` vs `centre`. I just prefer it to be consistent.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916847511
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,100 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map that includes the signature, db_name, idx_name, update_seq,
+ purge_seq, view_file_path, and pending_updates for an index.
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index.
Review Comment:
IdxState: State of an mrview index (`#mrst{}` record).
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916854411
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,100 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map that includes the signature, db_name, idx_name, update_seq,
Review Comment:
The inline representation of fields list is hard to read. Can you use something like:
```
Returns a state map that includes following fields:
- db_name
- idx_name
- ...
```
Also update documentation for other functions bellow.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923753098
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +280,152 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ State =
+ Info ++
+ [
+ {signature, Sig},
+ {db_name, DbName},
+ {idx_name, IdxState#mrst.idx_name},
+ {view_file_path, couch_mrview_util:index_file(DbName, Sig)}
+ ],
+ {ok, maps:from_list(State)};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(PidOrIdxState) ->
+ view_state(PidOrIdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ % reduce_funs contains tuples of {Name, ReduceFuns}
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ },
+ maps:put(Name, View, Acc)
+ end,
+ #{},
+ IdxState#mrst.views
+ ),
+ case ViewName of
+ undefined ->
+ {ok, MrViews};
+ _ ->
+ case maps:get(ViewName, MrViews) of
+ {badkey, Key} ->
+ io:format("No view named ~p was found.", [Key]),
+ {error, {badkey, Key}};
+ Value ->
+ {ok, #{ViewName => Value}}
+ end
+ end;
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+index_view_state(Pid) when is_pid(Pid) ->
+ index_view_state(Pid, undefined).
+
+index_view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = index_state(Pid),
+ {ok, ViewState} = view_state(Pid, ViewName),
+ {ok, maps:put(views, ViewState, IdxState)}.
+
+index_report(Pid) when is_pid(Pid) ->
+ case index_state(Pid) of
+ {ok, IdxState} ->
+ Sig = maps:get(signature, IdxState),
+ IdxState2 = maps:put(signature, couch_util:to_hex(Sig), IdxState),
+ % Convert collator versions to strings to print pretty
+ IdxState3 = convert_collator_versions_to_strings(IdxState2),
+ couch_debug:print_report_with_info_width(maps:to_list(IdxState3), 21);
Review Comment:
I've notices that `view_file_path` is quite long and truncated in the example of the output. Let's shorten `view_file_path` field by removing the common prefix.
```erlang
...
IdxState4 = maps:update_with(view_file_path, fun format_view_path/1, IdxState3).
...
end.
format_view_path(ViewFilePath) ->
BaseDir = config:get("couchdb", "view_index_dir"),
lists:flatten(string:replace(ViewFilePath, BaseDir ++ "/", "")).
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ...,
reduce_funs => ...,
def => ...,
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(State, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## View {name}
couch_debug:print_report(ParticularViewInfo)
### map
{def}
### reduce
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => { <---- it is either from map_names or reduce_funs
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_def => ... <- def
reduce_def => ... <--- a second element of tuples in reduce_funs
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(State, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## Index {idx_name} <---- taken from #mrst.idx_name
couch_debug:print_report(ParticularViewInfo)
### map {map_name} <--- taken from #mrview.map_names
{def}
### reduce {reduce_name} <- taken from #mrview.reduce_funs == ViewName
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882711045
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
Review Comment:
Please don't use `element/2`, the `element/2` will stop working when the record definition would change and we don't have compiler errors forcing us to fix this code. Use corresponding records directly.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882707436
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
Review Comment:
I don't think this is the best place for this function. It knows too much about index internals. Let's move it into https://github.com/apache/couchdb/blob/3.x/src/couch_mrview/src/couch_mrview_debug.erl
Also I think the API should be:
- `processes() -> [pid()]`
- `processes_info(Pid) -> {pid(), id(), map()}.` - not sure about id()
- `report(Pid) -> prints report` - you would call the function above to get all info and format it nicelly
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r898350755
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
Review Comment:
Should the following pattern match with `{value, {Key, Value}}` that `couch_util:get_value/3` uses?
```
{value,{mrview,0,3,0,
[<<"dbv1view">>],
[],<<"function(doc){emit(doc.id, 1);}">>,
{btree,<7954.955.0>,
{109,{2,[],24},51},
undefined,undefined,
fun couch_ejson_compare:less_json_ids/2,
#Fun<couch_mrview_util.14.69111627>,snappy},
```
I would think not since the parities of the tuples do not match.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916869505
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, InfoVal} where each InfoKey is unique
+ (unlike print_table/2).
+
Review Comment:
In addition to the description of the Report format. Could you provide an example which illustrates the shape of the data expected by this function?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement index report functions
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923937628
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,223 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map for an index that includes the following fields:
+ - collator_versions
+ - compact_running
+ - db_name
+ - idx_name
+ - language
+ - pending_updates
+ - purge_seq
+ - signature
+ - sizes
+ - update_options
+ - update_seq
+ - updater_running
+ - view_file_path
+ - waiting_clients
+ - waiting_commit
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map for a ViewName if specified or all views if not that includes the
+ following fields:
+ - btree_size
+ - def
+ - id_num
+ - options
+ - purge_seq
+ - reduce_funs
+ - update_seq
+
+ ---
+ ", []);
+help(index_view_state) ->
+ io:format("
+ index_view_state(Pid)
+ index_view_state(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the index state returned by index_state/1 and the view
+ state returned by view_state/2. Like view_state/2, a ViewName can be specified or not.
+
+ ---
+ ", []);
+help(index_report) ->
+ io:format("
+ index_report(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Prints a report for the index state of an mrview index that includes the following fields:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+
+ ---
+ ", []);
+help(view_report) ->
+ io:format("
+ view_report(PidOrIdxState)
+ view_report(Pid, ViewName)
+ view_report(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for a ViewName if specified or all views if not that includes the following
+ fields:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ \"dbv1view\"
+ |info | value
+ | btree_size | 51
+ | def | function(doc){emit(doc.id, 1);}
+ | id_num | 0
+ | options |
+ | purge_seq | 0
+ | reduce_funs |
+ | update_seq | 3
+ \"dbv2view\"
+ |info | value
+ | btree_size | 50
+ | def | function(doc){emit(doc.id, 2);}
+ | id_num | 1
+ | options |
+ | purge_seq | 0
+ | reduce_funs | _sum
+ | update_seq | 3
+
+ ---
+ ", []);
+help(index_view_report) ->
+ io:format("
+ index_view_report(Pid)
+ index_view_report(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for the index state and views of an mrview index. The report includes the following
+ fields for an index state:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+ The report also includes the following fields for a ViewName if specified or all views if not:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+ \"dbv1view\"
Review Comment:
Removed. Now only `dbv1view` is printed.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923678253
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
I like your approach. However I don't want to take `print_report/2` name. Since in the future I am planing to implement support for reports with more than 2 columns. In this case we would need to pass TableSpec in a second argument. Let's use your approach but name function differently.
```erlang
print_report(Report) ->
print_report_with_id_width(Report, 50).
print_report_with_id_width(Report, IdWidth) ->
TableSpec = [
{IdWidth, left, info},
{100, right, value}
],
....
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911189110
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
+ view_state(IdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
+ end,
+ #{},
+ IdxState#mrst.views
+ ),
+ case ViewName of
+ undefined ->
+ {ok, MrViews};
+ _ ->
+ {ok, #{ViewName => maps:get(ViewName, MrViews)}}
Review Comment:
I think we should display a proper error message for the case when user made a typo in `ViewName` and we couldn't find correspondent view in the state.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911186700
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
+ view_state(IdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ hd(MrView#mrview.reduce_funs)
Review Comment:
The return from the case branch here doesn't match the expected shape on line 153 `{Name, ReduceFuns}`. Would you mind adding a comment saying the `reduce_funs` contains `{Name, RedSrc}` tuples.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#issuecomment-1179065231
Looks really good. Just a few small improvements left. Mostly in documentation.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r915096360
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
Review Comment:
But I will save that for another PR.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r915088457
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
Review Comment:
I do not think they need to be written differently. We could move them to `couch_debug` and they could be used for other functions in different modules.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923663676
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
The following works perfectly and doesn't require much logic.
```
print_report(Report) ->
print_report(Report, 50).
print_report(Report, Width) ->
TableSpec = [
{Width, left, info},
{100, right, value}
],
io:format("~s~n", [format(TableSpec)]),
lists:map(
fun({InfoKey, Value}) ->
TableSpec1 = [
{Width, left, info},
{100, right, InfoKey}
],
io:format("~s~n", [table_row(InfoKey, 2, [{InfoKey, Value}], TableSpec1)])
end,
Report
).
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923711585
##########
src/couch/src/couch_debug.erl:
##########
@@ -289,12 +292,12 @@ help(print_linked_processes) ->
initial Pid to start from. The function doesn't recurse to pids
older than initial one. The output would look like similar to:
```
-couch_debug:print_linked_processes(whereis(couch_index_server)).
-name | reductions | message_queue_len | memory
-couch_index_server[<0.288.0>] | 478240 | 0 | 109696
- couch_index:init/1[<0.3520.22>] | 4899 | 0 | 109456
- couch_file:init/1[<0.886.22>] | 11973 | 0 | 67984
- couch_index:init/1[<0.3520.22>] | 4899 | 0 | 109456
+ couch_debug:print_linked_processes(whereis(couch_index_server)).
Review Comment:
The reason I indented it on the left is to help visually impaired people like myself to use bigger fonts. It causes additional indentation when you run `couch_debug:help(print_linked_processes)` in the `remsh`. With bigger font the table might not fit into a screen width and all formatting appear broken. It is not the case for this particular table, so I am fine with this change. Just wanted to provide rational for the previous choice of indentation.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r917050778
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, InfoVal} where each InfoKey is unique
+ (unlike print_table/2).
+
Review Comment:
As in the format the table prints in?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911199017
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
+ view_state(IdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
+ end,
+ #{},
+ IdxState#mrst.views
+ ),
+ case ViewName of
+ undefined ->
+ {ok, MrViews};
+ _ ->
+ {ok, #{ViewName => maps:get(ViewName, MrViews)}}
+ end;
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+index_view_state(Pid) when is_pid(Pid) ->
+ index_view_state(Pid, undefined).
+
+index_view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = index_state(Pid),
+ {ok, ViewState} = view_state(Pid, ViewName),
+ {ok, maps:put(views, ViewState, IdxState)}.
+
+index_report(Pid) when is_pid(Pid) ->
+ case index_state(Pid) of
+ {ok, IdxState} ->
+ couch_debug:print_report(maps:to_list(IdxState));
+ Error ->
+ Error
+ end.
+
+view_report(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_report(IdxState, undefined);
+view_report(IdxState) ->
+ view_report(IdxState, undefined).
+
+view_report(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_report(IdxState, ViewName);
+view_report(IdxState, ViewName) ->
+ case view_state(IdxState, ViewName) of
+ {ok, ViewState} ->
+ lists:foreach(
+ fun({Name, Info}) ->
+ io:format("~p~n", [binary_to_list(Name)]),
+ couch_debug:print_report(maps:to_list(Info))
+ end,
+ maps:to_list(ViewState)
Review Comment:
Are you planing to format ViewState as a table?
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
+ view_state(IdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
+ end,
+ #{},
+ IdxState#mrst.views
+ ),
+ case ViewName of
+ undefined ->
+ {ok, MrViews};
+ _ ->
+ {ok, #{ViewName => maps:get(ViewName, MrViews)}}
+ end;
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+index_view_state(Pid) when is_pid(Pid) ->
+ index_view_state(Pid, undefined).
+
+index_view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = index_state(Pid),
+ {ok, ViewState} = view_state(Pid, ViewName),
+ {ok, maps:put(views, ViewState, IdxState)}.
+
+index_report(Pid) when is_pid(Pid) ->
+ case index_state(Pid) of
+ {ok, IdxState} ->
+ couch_debug:print_report(maps:to_list(IdxState));
+ Error ->
+ Error
+ end.
+
+view_report(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_report(IdxState, undefined);
+view_report(IdxState) ->
+ view_report(IdxState, undefined).
+
+view_report(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_report(IdxState, ViewName);
+view_report(IdxState, ViewName) ->
+ case view_state(IdxState, ViewName) of
+ {ok, ViewState} ->
+ lists:foreach(
+ fun({Name, Info}) ->
+ io:format("~p~n", [binary_to_list(Name)]),
+ couch_debug:print_report(maps:to_list(Info))
+ end,
+ maps:to_list(ViewState)
+ );
+ Error ->
+ Error
+ end.
+
+index_view_report(Pid) when is_pid(Pid) ->
+ case index_view_state(Pid) of
+ {ok, State} ->
+ Views = maps:get(views, State),
+ couch_debug:print_report(maps:to_list(maps:without([views], State))),
+ lists:foreach(
+ fun({Name, Info}) ->
+ io:format("~n~p~n", [binary_to_list(Name)]),
+ couch_debug:print_report(maps:to_list(Info))
+ end,
+ maps:to_list(Views)
Review Comment:
Are you planing to format ViewState as a table?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911193840
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
Review Comment:
We can remove this clause. Because the first clause of the `view_state/2` would handle this case.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896072340
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
I am not sure whether we would need `map_names` key. It could be redundant. As I can see the `<view_name>` in a views map might be sufficient.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r889043058
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
+ Report = [
+ {signature, [{signature, couch_index_util:hexsig(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, element(6, IdxState)}]},
+ {reduce_funs, [{reduce_funs, element(1, element(10, IdxState))}]},
+ {def, [{def, element(7, lists:nth(1, element(11, IdxState)))}]},
Review Comment:
You use `lists:nth(1...` to get the first element of a list. This is not idiomatic in erlang for two reasons.
1. There is a specialized function to return first element of a list `hd/1`.
2. The lists in Erlang is represented as nested structure and `lists:nth` recursively traverses the structure. This is not very efficient. On the other hand `hd/1` is in the core of Erlang and implemented in `C`. Which is order of magnitude more efficient.
For these reasons the `lists:nth(1...` is an anti-pattern.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882712227
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
+ Report = [
+ {signature, [{signature, couch_index_util:hexsig(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, element(6, IdxState)}]},
+ {reduce_funs, [{reduce_funs, element(1, element(10, IdxState))}]},
+ {def, [{def, element(7, lists:nth(1, element(11, IdxState)))}]},
Review Comment:
`lists:nth(1` -> `hd(Something)`
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r904189500
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
I have made some progress on viewing reports for multiple views.
```
> couch_mrview_debug:view_report(couch_mrview_debug:get_indexer_pids(), list_to_binary("dbv1view")).
| info | value
| db_name | shards/00000000-ffffffff/dbv1.1655922592
| idx_name | _design/dbv1ddoc
| pending_updates | 0
| purge_seq | 0
| signature | 917d560af42cb62fcaccc52ca8ddcb7d
| update_seq | 3
| view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1655922592_design/mrview/917d560af42cb62fcaccc52ca8ddcb7d.view
"dbv1view"
| info | value
| btree_size | 51
| def | function(doc){emit(doc.id, 1);}
| id_num | 0
| options |
| purge_seq | 0
| reduce_funs |
| update_seq | 3
ok
```
and
```
> couch_mrview_debug:view_report(couch_mrview_debug:get_indexer_pids()).
| info | value
| db_name | shards/00000000-ffffffff/dbv1.1655922592
| idx_name | _design/dbv1ddoc
| pending_updates | 0
| purge_seq | 0
| signature | 917d560af42cb62fcaccc52ca8ddcb7d
| update_seq | 3
| view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1655922592_design/mrview/917d560af42cb62fcaccc52ca8ddcb7d.view
"dbv1view"
| info | value
| btree_size | 51
| def | function(doc){emit(doc.id, 1);}
| id_num | 0
| options |
| purge_seq | 0
| reduce_funs |
| update_seq | 3
"dbv2view"
| info | value
| btree_size | 50
| def | function(doc){emit(doc.id, 2);}
| id_num | 1
| options |
| purge_seq | 0
| reduce_funs | _count;
| update_seq | 3
ok
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r904189500
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
I have made some progress on viewing reports for multiple views.
```
| info | value
| db_name | shards/00000000-ffffffff/dbv1.1655922592
| idx_name | _design/dbv1ddoc
| pending_updates | 0
| purge_seq | 0
| signature | 917d560af42cb62fcaccc52ca8ddcb7d
| update_seq | 3
| view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1655922592_design/mrview/917d560af42cb62fcaccc52ca8ddcb7d.view
| info | value
| btree_size | 50
| def | function(doc){emit(doc.id, 2);}
| id_num | 1
| options |
| purge_seq | 0
| reduce_funs | _count
| update_seq | 3
| view_name | dbv2view
| info | value
| btree_size | 51
| def | function(doc){emit(doc.id, 1);}
| id_num | 0
| options |
| purge_seq | 0
| reduce_funs |
| update_seq | 3
| view_name | dbv1view
```
Going to look into refactoring it into some of the functions you've mentioned.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r885820024
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
Review Comment:
The API will be `view_report(Pid) -> Prints report`. The first couple of lines in the function will be removed. They are solely for quick debugging right now.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891648241
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +63,27 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_report(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ Report = [
+ {signature, [{signature, couch_util:to_hex(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, IdxState#mrst.idx_name}]},
+ {reduce_funs, [{reduce_funs, MrView#mrview.reduce_funs}]},
+ {def, [{def, MrView#mrview.def}]},
+ {update_seq, [{update_seq, IdxState#mrst.update_seq}]},
+ {purge_seq, [{purge_seq, IdxState#mrst.purge_seq}]},
+ {view_file_path, [{view_file_path, couch_mrview_util:index_file(DbName, Sig)}]},
+ {pending_updates, [lists:keyfind(pending_updates, 1, Info)]}
+ ],
+ couch_debug:print_report(Report);
Review Comment:
Please separate getting the report as a data structure and printing that structure to the console. The main use case for this separation is to allow building complex logic on top of data structures returned from `_debug` modules.
```
view_state(Pid) -> %% {ok, Report} | {error, Reason}
...
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} -> couch_debug:print_report(Report);
...
```
In this case `view_state/1` is useful by itself in cases when we need a data structure as a result.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891694259
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
It is possible to build it in `print_report` instead. I will move it and simplify the `Report` data structure.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r895996877
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
Review Comment:
In CouchDB codebase we use `couch_util:get_value/{2,3}.` and in very few cases `proplists:get_value/{2,3}` (when we want to avoid dependency on `couch` app). The `couch_mrview` uses `couch_util:get_value` over the place. Please use it as well for consistency.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r898350755
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
Review Comment:
Should the following pattern match with `{value, {Key, Value}}` that `couch_util:get_value/3` uses?
```
{value,{mrview,0,3,0,
[<<"dbv1view">>],
[],<<"function(doc){emit(doc.id, 1);}">>,
{btree,<7954.955.0>,
{109,{2,[],24},51},
undefined,undefined,
fun couch_ejson_compare:less_json_ids/2,
#Fun<couch_mrview_util.14.69111627>,snappy},
[]}}
```
I would think not since the parities of the tuples do not match.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ...,
reduce_funs => ...,
def => ...,
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(State, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## Index {idx_name} <---- taken from #mrst.idx_name
couch_debug:print_report(ParticularViewInfo)
### map {map_name} <--- taken from #mrview.map_names
{def}
### reduce {reduce_name} <- taken from #mrview.reduce_funs == ViewName
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911205333
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
Review Comment:
I think we should return `Sig` as is and call `couch_util:to_hex/1` only from `xxx_report` functions. The goal for `xxx_state` functions is to allow operators write shell functions to manipulate the data structures to extract information from the state.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak merged pull request #4033: Implement index report functions
Posted by GitBox <gi...@apache.org>.
iilyak merged PR #4033:
URL: https://github.com/apache/couchdb/pull/4033
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r912256762
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
Review Comment:
What do you mean by consistent. `view_state/2` returns a map of either
```
#{
ViewName1 => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
},
ViewName2 => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
},
...
}
```
if no `ViewName` is specified or
```
#{
ViewName => #{
id_num => MrView#mrview.id_num,
update_seq => MrView#mrview.update_seq,
purge_seq => MrView#mrview.purge_seq,
reduce_funs => ReduceFuns,
def => MrView#mrview.def,
btree_size => couch_btree:size(MrView#mrview.btree),
options => MrView#mrview.options
}
}
```
if a `ViewName` is specified.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r916842651
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
+ TableSpec = [
+ {50, centre, info},
+ {100, right, value}
+ ],
+ io:format("~s~n", [format(TableSpec)]),
+ lists:map(
+ fun({InfoKey, Value}) ->
+ TableSpec1 = [
Review Comment:
If you would follow the suggestion about providing `print_report/2` version of the function you wouldn't be able to have a second copy of a TableSpec. One option is to derive second TableSpec from the first one. I though about something like.
```
TableSpec1 = lists:keymap(fun
(value) -> InfoKey;
(X) -> X
end, 3, TableSpec).
```
However there is a better solution (in terms of performance). This function assumes (as it is currently written in your PR) that the table has only 2 columns. Therefore we can pattern match on the shape:
```
print_report(Report, [Column1, {W2, A2, _}] = TableSpec) ->
io:format("~s~n", [format(TableSpec)]),
lists:map(
fun({InfoKey, Value}) ->
TableSpec1 = [Column1, {W2, A2, InfoKey}],
io:format("~s~n", [table_row(InfoKey, 2, [{InfoKey, Value}], TableSpec1)])
end,
Report
);
print_report(_Report, TableSpec) when is_list(TableSpec) ->
throw({incorrect_spec, lists:flatten(io_lib:format("We expect a table with 2 rows, but got ~p", [length(TableSpec)]))});
print_report(_Report, TableSpec) ->
throw({incorrect_spec, "TableSpec must be a list, i.e. [{20, centre, info},{100, right, value}]"}}).
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r917097785
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, InfoVal} where each InfoKey is unique
+ (unlike print_table/2).
+
Review Comment:
yes
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923678253
##########
src/couch/src/couch_debug.erl:
##########
@@ -891,6 +904,23 @@ print_table(Rows, TableSpec) ->
),
ok.
+print_report(Report) ->
Review Comment:
I like your approach. However I don't want to take `print_report/2` name. Since in the future I am planing to implement support for reports with more than 2 columns. In this case we would need to pass TableSpec in a second argument. Let's use your approach but name function differently.
```erlang
print_report(Report) ->
print_report_with_id_with(Report, 50).
print_report_with_id_with(Report, IdWidth) ->
TableSpec = [
{IdWidth, left, info},
{100, right, value}
],
....
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923716239
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,223 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map for an index that includes the following fields:
+ - collator_versions
+ - compact_running
+ - db_name
+ - idx_name
+ - language
+ - pending_updates
+ - purge_seq
+ - signature
+ - sizes
+ - update_options
+ - update_seq
+ - updater_running
+ - view_file_path
+ - waiting_clients
+ - waiting_commit
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map for a ViewName if specified or all views if not that includes the
+ following fields:
+ - btree_size
+ - def
+ - id_num
+ - options
+ - purge_seq
+ - reduce_funs
+ - update_seq
+
+ ---
+ ", []);
+help(index_view_state) ->
+ io:format("
+ index_view_state(Pid)
+ index_view_state(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the index state returned by index_state/1 and the view
+ state returned by view_state/2. Like view_state/2, a ViewName can be specified or not.
+
+ ---
+ ", []);
+help(index_report) ->
+ io:format("
+ index_report(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Prints a report for the index state of an mrview index that includes the following fields:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+
+ ---
+ ", []);
+help(view_report) ->
+ io:format("
+ view_report(PidOrIdxState)
+ view_report(Pid, ViewName)
+ view_report(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for a ViewName if specified or all views if not that includes the following
+ fields:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ \"dbv1view\"
+ |info | value
+ | btree_size | 51
+ | def | function(doc){emit(doc.id, 1);}
+ | id_num | 0
+ | options |
+ | purge_seq | 0
+ | reduce_funs |
+ | update_seq | 3
+ \"dbv2view\"
+ |info | value
+ | btree_size | 50
+ | def | function(doc){emit(doc.id, 2);}
+ | id_num | 1
+ | options |
+ | purge_seq | 0
+ | reduce_funs | _sum
+ | update_seq | 3
+
+ ---
+ ", []);
+help(index_view_report) ->
+ io:format("
+ index_view_report(Pid)
+ index_view_report(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for the index state and views of an mrview index. The report includes the following
+ fields for an index state:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+ The report also includes the following fields for a ViewName if specified or all views if not:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+ \"dbv1view\"
Review Comment:
Do we output the escaped quotes in the terminal? If so I think we should change it.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r923869125
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -34,7 +50,223 @@ help(view_signature) ->
io:format("
view_signature(ShardName, DDocName)
--------------
+
Returns a view signature for given ddoc for a given (non clustered) database.
+
+ ---
+ ", []);
+help(index_state) ->
+ io:format("
+ index_state(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Returns a state map for an index that includes the following fields:
+ - collator_versions
+ - compact_running
+ - db_name
+ - idx_name
+ - language
+ - pending_updates
+ - purge_seq
+ - signature
+ - sizes
+ - update_options
+ - update_seq
+ - updater_running
+ - view_file_path
+ - waiting_clients
+ - waiting_commit
+
+ ---
+ ", []);
+help(view_state) ->
+ io:format("
+ view_state(PidOrIdxState)
+ view_state(Pid, ViewName)
+ view_state(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map for a ViewName if specified or all views if not that includes the
+ following fields:
+ - btree_size
+ - def
+ - id_num
+ - options
+ - purge_seq
+ - reduce_funs
+ - update_seq
+
+ ---
+ ", []);
+help(index_view_state) ->
+ io:format("
+ index_view_state(Pid)
+ index_view_state(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Returns a state map that includes the index state returned by index_state/1 and the view
+ state returned by view_state/2. Like view_state/2, a ViewName can be specified or not.
+
+ ---
+ ", []);
+help(index_report) ->
+ io:format("
+ index_report(Pid)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+
+ Prints a report for the index state of an mrview index that includes the following fields:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+
+ ---
+ ", []);
+help(view_report) ->
+ io:format("
+ view_report(PidOrIdxState)
+ view_report(Pid, ViewName)
+ view_report(IdxState, ViewName)
+ --------------
+
+ PidOrIdxState: Pid of couch_index:init/1, specifically an mrview index, or the state
+ of an mrview index.
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ IdxState: State of an mrview index (#mrst{} record).
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for a ViewName if specified or all views if not that includes the following
+ fields:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ \"dbv1view\"
+ |info | value
+ | btree_size | 51
+ | def | function(doc){emit(doc.id, 1);}
+ | id_num | 0
+ | options |
+ | purge_seq | 0
+ | reduce_funs |
+ | update_seq | 3
+ \"dbv2view\"
+ |info | value
+ | btree_size | 50
+ | def | function(doc){emit(doc.id, 2);}
+ | id_num | 1
+ | options |
+ | purge_seq | 0
+ | reduce_funs | _sum
+ | update_seq | 3
+
+ ---
+ ", []);
+help(index_view_report) ->
+ io:format("
+ index_view_report(Pid)
+ index_view_report(Pid, ViewName)
+ --------------
+
+ Pid: Pid of couch_index:init/1, specifically an mrview index.
+ ViewName: Name of the view to be queried or undefined.
+
+ Prints a report for the index state and views of an mrview index. The report includes the following
+ fields for an index state:
+ - signature
+ - db_name
+ - idx_name
+ - update_seq
+ - purge_seq
+ - view_file_path
+ - pending_updates
+ The report also includes the following fields for a ViewName if specified or all views if not:
+ - id_num
+ - update_seq
+ - purge_seq
+ - reduce_funs
+ - def
+ - btree_size
+ - options
+
+ The output will look similar to:
+
+ |info | value
+ | collator_versions | 153.112
+ | compact_running | false
+ | db_name | shards/00000000-ffffffff/dbv1.1658165106
+ | idx_name | _design/dbv1ddoc
+ | language | javascript
+ | pending_updates | 0
+ | purge_seq | 0
+ | signature | a967fb72089e71e870f790f32bcc6a55
+ | sizes | {[{file,4264},{active,163},{external,51}]}
+ | update_options |
+ | update_seq | 3
+ | updater_running | false
+ | view_file_path |1/data/.shards/00000000-ffffffff/dbv1.1658165106_design/mrview/a967fb72089e71e870f790f32bcc6a55.view
+ | waiting_clients | 0
+ | waiting_commit | false
+ \"dbv1view\"
Review Comment:
Yes, the quotes are outputted to the terminal. To be specific, `"dbv1view"` will be printed to the terminal, not `dbv1view`. I can remove the quotes.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#issuecomment-1179064043
> I also want to clarify the need for a print_report function on top of the existing print_table function. If the print_table function is used, the following is printed:
I think `print_report` output is cleaner and shorter.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r905347617
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
I have added more function clauses for when a `Pid` or an `IdxState` is given. I have also added the ability to retrieve just the index state, the view state, or the index + view state and have done the same regarding printing reports.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r895986586
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
I think docs are no longer represent the data structure expected as an argument.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r896069919
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +78,36 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+view_state(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ MrView = lists:keyfind(mrview, 1, IdxState#mrst.views),
Review Comment:
You are making an assumption that the user of the function is only interested in a very first view out of possibly many.
We could have `view_state(Pid, Name)`. However in this case we need an additional function to list views `list_views(Pid)`. Another option would be to return information about multiple views.
If we would go the second route the data structure returned from the function would look like (assuming we convert to map)
```
#{
signature => ...,
db_name => ...,
idx_name => ...,
update_seq => ...,
purge_seq => ...,
view_file_path => ...,
pending_updates => ...,
views =>
#{ <view_name> => {
id_num => ...,
update_seq => ...,
purge_seq => ...,
map_names => ...,
reduce_funs => ...,
def => ...,
btree_size => btree:size(BTree),
options=> ...
}
}
}
},
```
In this case you would need to do extra work in view_report to print views individually after common info
```
view_report(Pid) ->
case view_state(Pid) of
{ok, Report} ->
{value, {views, Views}, Rest} = lists:keytake(views, 1, Report),
couch_debug:print_report(Rest),
lists:map(fun print_view_report/1, Views);
Error ->
Error
end.
```
Maybe we should have both variants:
```
index_state(Pid) -> #{views => #{}}
view_state(IndexState | Pid, ViewName) -> #{}
list_views(IndexState | Pid) -> []
index_report(Pid) when is_pid(Pid) ->
index_report(index_state(Pid), ViewName);
index_report(State) ->
ReportWithoutViewsInfo = get it from State,
couch_debug:print_report(ReportWithoutViewsInfo),
# views
Views = get it from State,
lists:foreach(fun(Name) ->
view_report(State, Name) end, list_views(State))
).
view_report(Pid, Name) when is_pid(Pid) ->
view_report(index_state(Pid), Name);
view_report(PidOrState, Name) ->
couch_debug:print_report(ReportWithoutViewsInfo),
print_view_report(Pid, Name).
print_view_report(Pid, ViewName) when is_pid(Pid) ->
print_view_report(index_state(Pid), ViewName);
print_view_report(State, ViewName) ->
## View {name}
couch_debug:print_report(PartivularViewInfo)
### map
{def}
### reduce
{reduce} <--- taken from #mrview.reduce_funs
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r911214163
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,154 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => couch_util:to_hex(Sig),
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, undefined);
+view_state(IdxState) ->
Review Comment:
I suggest to always return a list from `view_state/2` to make the return value of the function consistent. Then we would have to unwrap in this clause.
```
view_state(PidOrIdxState) ->
case view_state(IdxState, undefined) of
{ok, [Res]} ->
{ok, Res};
Error ->
Error
end.
view_state(IdxState, ViewName) ->
....
case ViewName of
undefined ->
{ok, MrViews};
_ ->
{ok, [#{ViewName => maps:get(ViewName, MrViews)}]
...
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r915174970
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
Review Comment:
I have removed `first_call`, `find_by_first_call`, and `get_indexer_pid` from the PR. They were initially only added for testing purposes.
##########
src/couch_mrview/src/couch_mrview_debug.erl:
##########
@@ -48,3 +86,159 @@ view_signature(DbName, DDocName) ->
{ok, DDoc} = couch_db:open_doc_int(Db, <<"_design/", DDocName/binary>>, []),
{ok, IdxState} = couch_mrview_util:ddoc_to_mrst(DDocName, DDoc),
couch_util:to_hex(IdxState#mrst.sig).
+
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+get_indexer_pid() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ hd(IndexerPids).
+
+index_state(Pid) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case IdxState of
+ #mrst{} ->
+ {ok, Info} = couch_index:get_info(Pid),
+ Sig = IdxState#mrst.sig,
+ DbName = IdxState#mrst.db_name,
+ {pending_updates, PendingUpdates} = lists:keyfind(pending_updates, 1, Info),
+ State = #{
+ signature => Sig,
+ db_name => DbName,
+ idx_name => IdxState#mrst.idx_name,
+ update_seq => IdxState#mrst.update_seq,
+ purge_seq => IdxState#mrst.purge_seq,
+ view_file_path => couch_mrview_util:index_file(DbName, Sig),
+ pending_updates => PendingUpdates
+ },
+ {ok, State};
+ _ ->
+ {error, not_mrview_index}
+ end.
+
+view_state(PidOrIdxState) ->
+ view_state(PidOrIdxState, undefined).
+
+view_state(Pid, ViewName) when is_pid(Pid) ->
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ view_state(IdxState, ViewName);
+view_state(IdxState, ViewName) ->
+ case IdxState of
+ #mrst{} ->
+ MrViews = lists:foldl(
+ fun(MrView, Acc) ->
+ {Name, ReduceFuns} =
+ case MrView#mrview.reduce_funs of
+ [] ->
+ {hd(MrView#mrview.map_names), []};
+ _ ->
+ % reduce_funs contains tuples of {Name, ReduceFuns}
+ hd(MrView#mrview.reduce_funs)
+ end,
+ View = #{
+ Name => #{
+ id_num => MrView#mrview.id_num,
+ update_seq => MrView#mrview.update_seq,
+ purge_seq => MrView#mrview.purge_seq,
+ reduce_funs => ReduceFuns,
+ def => MrView#mrview.def,
+ btree_size => couch_btree:size(MrView#mrview.btree),
+ options => MrView#mrview.options
+ }
+ },
+ maps:merge(View, Acc)
Review Comment:
Fixed.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r885820485
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
+ mrst ->
+ {ok, Info} = couch_index:get_info(Pid),
+ {signature, Sig} = lists:keyfind(signature, 1, Info),
+ DbName = element(5, IdxState),
+ Report = [
+ {signature, [{signature, couch_index_util:hexsig(Sig)}]},
+ {db_name, [{db_name, DbName}]},
+ {idx_name, [{idx_name, element(6, IdxState)}]},
+ {reduce_funs, [{reduce_funs, element(1, element(10, IdxState))}]},
+ {def, [{def, element(7, lists:nth(1, element(11, IdxState)))}]},
Review Comment:
I don't understand this comment.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r882708935
##########
src/couch/src/couch_debug.erl:
##########
@@ -794,6 +795,68 @@ id("couch_file:init" ++ _, Pid, _Props) ->
id(_IdStr, _Pid, _Props) ->
"".
+first_call(Pid) ->
+ IC =
+ case process_info(Pid, initial_call) of
+ {initial_call, IC0} -> IC0;
+ undefined -> undefined
+ end,
+ Dict =
+ case process_info(Pid, dictionary) of
+ {dictionary, Dict0} -> Dict0;
+ undefined -> []
+ end,
+ MaybeCall = proplists:get_value('$initial_call', Dict, IC),
+ proplists:get_value(initial_call, Dict, MaybeCall).
+
+find_by_first_call(Module, Function) ->
+ FilterPids = fun(Pid) ->
+ case first_call(Pid) of
+ {Module, Function, _} -> true;
+ _ -> false
+ end
+ end,
+ lists:filter(FilterPids, processes()).
+
+view_report() ->
+ IndexerPids = find_by_first_call(couch_index, init),
+ Pid = lists:nth(1, IndexerPids),
+ {ok, IdxState} = couch_index:get_state(Pid, 0),
+ case element(1, IdxState) of
Review Comment:
The `#mrst` record should be available once you move it into `couch_mrview_debug.erl`
```
case IdxState of
#mrst{} ->
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a diff in pull request #4033: Implement view_report function
Posted by GitBox <gi...@apache.org>.
iilyak commented on code in PR #4033:
URL: https://github.com/apache/couchdb/pull/4033#discussion_r891652010
##########
src/couch/src/couch_debug.erl:
##########
@@ -326,6 +328,17 @@ help(print_table) ->
- TableSpec: List of either {Value} or {Width, Align, Value}
where Align is either left/center/right.
+ ---
+ ", []);
+help(print_report) ->
+ io:format("
+ print_report(Report)
+ --------------------------------
+
+ Print a report in table form.
+ - Report: List of {InfoKey, [{InfoKey, InfoVal}]} where each InfoKey is unique
Review Comment:
Why do we need to specify `InfoKey` twice? Can we construct it in `print_report`?
```
lists:map(
fun({InfoKey, Value}) ->
TableSpec1 = [
{50, centre, info},
{100, right, InfoKey}
],
io:format("~s~n", [table_row(InfoKey, 2, {InfoKey, Value}, TableSpec1)])
end,
Report
).
```
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org