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 2020/03/23 05:58:34 UTC
[GitHub] [couchdb] tonysun83 opened a new pull request #2706: [WIP] add info
endpoint for fdb stored views
tonysun83 opened a new pull request #2706: [WIP] add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706
## Overview
This is a WIP to add the `_info` endpoint for views stored in fdb. Currently, I am unsure of the backwards compatibility for some of the fields returned by traditional couchdb views.
Info can be divided into 3 categories:
1) Status of the build - In traditional views, we looked at
**a) the update seq of the view**
b) committed seq
c) number of clients waiting to commit
d) number of clients waiting
e) if the indexer is still running
We can determine **a)** by calling `couch_views_fdb:get_update_seq/2` (note it's a binary instead of couch_mrview's `update_seq` values which are integers). In traditional views, **b)-d)** refers the number of workers in a queue that are waiting to start indexing and then committing. I'm not sure how this transfers over to fdb as workers are part of the couch_jobs framework and commits occur for very transaction. For e), I looked at `couch_jobs` to see if crafting the `job_id` can give us the state of the job. But it requires a transaction so not sure if this is possible either. (need to verify with @nickva ).
2) Size of the Index - Disk size does not seem applicable. For data size, `couch_view` has a `couch_views_fdb:get_kv_size/2` function for each view built. I don't think the subcategories of size are applicable either: `file`, `external`, `active`.
Compaction - Not Applicable.
## Testing recommendations
Added a new `_info` endpoint test.
## Related Issues or Pull Requests
<!-- If your changes affects multiple components in different
repositories please put links to those issues or pull requests here. -->
## Checklist
- [ ] Code is written and works correctly
- [X] 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2706: [WIP]
add info endpoint for fdb stored views
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2706: [WIP] add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#discussion_r396566734
##########
File path: src/chttpd/src/chttpd_db.erl
##########
@@ -368,7 +368,7 @@ bad_action_req(#httpd{path_parts=[_, _, Name|FileNameParts]}=Req, Db, _DDoc) ->
handle_design_info_req(#httpd{method='GET'}=Req, Db, #doc{} = DDoc) ->
[_, _, Name, _] = Req#httpd.path_parts,
- {ok, GroupInfoList} = fabric:get_view_group_info(Db, DDoc),
+ {ok, GroupInfoList} = couch_views:get_views_info(Db, DDoc),
Review comment:
`get_views_info` is a bit awkward since we're getting a singular info object back. How about just `get_info` instead?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2706: add
info endpoint for fdb stored views
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#discussion_r396613371
##########
File path: src/couch_views/test/couch_views_info_test.erl
##########
@@ -0,0 +1,134 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_info_test).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include_lib("couch_mrview/include/couch_mrview.hrl").
+-include_lib("fabric/test/fabric2_test.hrl").
+
+-define(MAP_FUN1, <<"map_fun1">>).
+
+setup() ->
+ Ctx = test_util:start_couch([
+ fabric,
+ couch_jobs,
+ couch_js,
+ couch_views
+ ]),
+ Ctx.
+
+cleanup(Ctx) ->
+ test_util:stop_couch(Ctx).
+
+foreach_setup() ->
+ {ok, Db} = fabric2_db:create(?tempdb(), [{user_ctx, ?ADMIN_USER}]),
+ DDoc = create_ddoc(),
+ Doc1 = doc(0, 1),
+
+ {ok, _} = fabric2_db:update_doc(Db, DDoc, []),
+ {ok, _} = fabric2_db:update_doc(Db, Doc1, []),
+
+ run_query(Db, DDoc, ?MAP_FUN1),
+ {ok, Info} = couch_views:get_info(Db, DDoc),
+ {Db, Info}.
+
+foreach_teardown({Db, _}) ->
+ meck:unload(),
+ ok = fabric2_db:delete(fabric2_db:name(Db), []).
+
+views_info_test_() ->
+ {
+ "Views index info test",
+ {
+ setup,
+ fun setup/0,
+ fun cleanup/1,
+ {
+ foreach,
+ fun foreach_setup/0,
+ fun foreach_teardown/1,
+ [
+ fun sig_is_binary/1,
+ fun language_is_js/1,
+ fun update_seq_is_binary/1,
+ fun updater_running_is_boolean/1,
+ fun data_size_is_non_neg_int/1,
+ fun active_size_is_non_neg_int/1,
+ fun update_opts_is_bin_list/1
+ ]
+ }
+ }
+ }.
+
+sig_is_binary({_, Info}) ->
+ ?_assert(is_binary(prop(signature, Info))).
+
+language_is_js({_, Info}) ->
+ ?_assertEqual(<<"javascript">>, prop(language, Info)).
+
+data_size_is_non_neg_int({_, Info}) ->
+ ?_assert(check_non_neg_int(data_size, Info)).
Review comment:
This should fail, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] tonysun83 commented on a change in pull request #2706:
add info endpoint for fdb stored views
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on a change in pull request #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#discussion_r396620134
##########
File path: src/couch_views/test/couch_views_info_test.erl
##########
@@ -0,0 +1,134 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_info_test).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include_lib("couch_mrview/include/couch_mrview.hrl").
+-include_lib("fabric/test/fabric2_test.hrl").
+
+-define(MAP_FUN1, <<"map_fun1">>).
+
+setup() ->
+ Ctx = test_util:start_couch([
+ fabric,
+ couch_jobs,
+ couch_js,
+ couch_views
+ ]),
+ Ctx.
+
+cleanup(Ctx) ->
+ test_util:stop_couch(Ctx).
+
+foreach_setup() ->
+ {ok, Db} = fabric2_db:create(?tempdb(), [{user_ctx, ?ADMIN_USER}]),
+ DDoc = create_ddoc(),
+ Doc1 = doc(0, 1),
+
+ {ok, _} = fabric2_db:update_doc(Db, DDoc, []),
+ {ok, _} = fabric2_db:update_doc(Db, Doc1, []),
+
+ run_query(Db, DDoc, ?MAP_FUN1),
+ {ok, Info} = couch_views:get_info(Db, DDoc),
+ {Db, Info}.
+
+foreach_teardown({Db, _}) ->
+ meck:unload(),
+ ok = fabric2_db:delete(fabric2_db:name(Db), []).
+
+views_info_test_() ->
+ {
+ "Views index info test",
+ {
+ setup,
+ fun setup/0,
+ fun cleanup/1,
+ {
+ foreach,
+ fun foreach_setup/0,
+ fun foreach_teardown/1,
+ [
+ fun sig_is_binary/1,
+ fun language_is_js/1,
+ fun update_seq_is_binary/1,
+ fun updater_running_is_boolean/1,
+ fun data_size_is_non_neg_int/1,
+ fun active_size_is_non_neg_int/1,
+ fun update_opts_is_bin_list/1
+ ]
+ }
+ }
+ }.
+
+sig_is_binary({_, Info}) ->
+ ?_assert(is_binary(prop(signature, Info))).
+
+language_is_js({_, Info}) ->
+ ?_assertEqual(<<"javascript">>, prop(language, Info)).
+
+data_size_is_non_neg_int({_, Info}) ->
+ ?_assert(check_non_neg_int(data_size, Info)).
Review comment:
It's just checking that it's >= 0 right?
```
Views index info test
couch_views_info_test:75: sig_is_binary...ok
couch_views_info_test:78: language_is_js...ok
couch_views_info_test:90: update_seq_is_binary...ok
couch_views_info_test:87: updater_running_is_boolean...ok
couch_views_info_test:81: data_size_is_non_neg_int...ok
couch_views_info_test:84: active_size_is_non_neg_int...ok
couch_views_info_test:94: update_opts_is_bin_list...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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2706: add
info endpoint for fdb stored views
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#discussion_r396632438
##########
File path: src/couch_views/test/couch_views_info_test.erl
##########
@@ -0,0 +1,134 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_views_info_test).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include_lib("couch_mrview/include/couch_mrview.hrl").
+-include_lib("fabric/test/fabric2_test.hrl").
+
+-define(MAP_FUN1, <<"map_fun1">>).
+
+setup() ->
+ Ctx = test_util:start_couch([
+ fabric,
+ couch_jobs,
+ couch_js,
+ couch_views
+ ]),
+ Ctx.
+
+cleanup(Ctx) ->
+ test_util:stop_couch(Ctx).
+
+foreach_setup() ->
+ {ok, Db} = fabric2_db:create(?tempdb(), [{user_ctx, ?ADMIN_USER}]),
+ DDoc = create_ddoc(),
+ Doc1 = doc(0, 1),
+
+ {ok, _} = fabric2_db:update_doc(Db, DDoc, []),
+ {ok, _} = fabric2_db:update_doc(Db, Doc1, []),
+
+ run_query(Db, DDoc, ?MAP_FUN1),
+ {ok, Info} = couch_views:get_info(Db, DDoc),
+ {Db, Info}.
+
+foreach_teardown({Db, _}) ->
+ meck:unload(),
+ ok = fabric2_db:delete(fabric2_db:name(Db), []).
+
+views_info_test_() ->
+ {
+ "Views index info test",
+ {
+ setup,
+ fun setup/0,
+ fun cleanup/1,
+ {
+ foreach,
+ fun foreach_setup/0,
+ fun foreach_teardown/1,
+ [
+ fun sig_is_binary/1,
+ fun language_is_js/1,
+ fun update_seq_is_binary/1,
+ fun updater_running_is_boolean/1,
+ fun data_size_is_non_neg_int/1,
+ fun active_size_is_non_neg_int/1,
+ fun update_opts_is_bin_list/1
+ ]
+ }
+ }
+ }.
+
+sig_is_binary({_, Info}) ->
+ ?_assert(is_binary(prop(signature, Info))).
+
+language_is_js({_, Info}) ->
+ ?_assertEqual(<<"javascript">>, prop(language, Info)).
+
+data_size_is_non_neg_int({_, Info}) ->
+ ?_assert(check_non_neg_int(data_size, Info)).
Review comment:
Ah, I should have looked harder. You need to remove `data_size` as that regresses our API changes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] davisp commented on a change in pull request #2706: add
info endpoint for fdb stored views
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#discussion_r396614121
##########
File path: src/couch_views/src/couch_views.erl
##########
@@ -73,6 +74,43 @@ build_indices(#{} = Db, DDocs) when is_list(DDocs) ->
end
end, DDocs).
+get_info(Db, DDoc) ->
Review comment:
Getting into the super nitty gritty on style, but its two empty lines between functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] tonysun83 commented on issue #2706: add info endpoint for
fdb stored views
Posted by GitBox <gi...@apache.org>.
tonysun83 commented on issue #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706#issuecomment-602721634
I decided to use `couch_mrview_index:get/2` even though we aren't supporting `Partitioned`. It's saves additional code that's going to be very similar and partitioned will only be returned on the odd chance it's incorrectly specified for indexing. It won't break anything but instead just return the value. The new layout:
```
{
"name": "foo",
"view_index": {
"language": "javascript",
"signature": "b7bf9f8b5adefa4b23c4cf60448d8fad",
"sizes": {
"active": 0
},
"update_seq": "",
"updater_running": false,
"data_size": 0,
"update_options": []
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [couchdb] tonysun83 merged pull request #2706: add info endpoint
for fdb stored views
Posted by GitBox <gi...@apache.org>.
tonysun83 merged pull request #2706: add info endpoint for fdb stored views
URL: https://github.com/apache/couchdb/pull/2706
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services