[GitHub] [couchdb] garrensmith commented on a change in pull request #3003: add active_tasks for view builds using version stamps

[GitBox <gi...@apache.org>]

garrensmith commented on a change in pull request #3003:
URL: https://github.com/apache/couchdb/pull/3003#discussion_r454200239



##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -541,3 +556,25 @@ key_size_limit() ->
 
 value_size_limit() ->
     config:get_integer("couch_views", "value_size_limit", ?VALUE_SIZE_LIMIT).
+
+
+update_tasks(NumChanges, false) ->

Review comment:
       Can you rename this to `update_active_tasks`

##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -231,10 +231,24 @@ maybe_set_build_status(TxDb, Mrst1, _ViewVS, State) ->
 get_update_start_state(TxDb, Mrst, #{db_seq := undefined} = State) ->
     ViewVS = couch_views_fdb:get_creation_vs(TxDb, Mrst),
     ViewSeq = couch_views_fdb:get_update_seq(TxDb, Mrst),
+    DbSeq = fabric2_db:get_update_seq(TxDb),
+    DbVS = fabric2_fdb:seq_to_vs(DbSeq),

Review comment:
       Can you move all the related active_tasks logic into its own function. Maybe something like `active_tasks_start`.
   

##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -541,3 +556,25 @@ key_size_limit() ->
 
 value_size_limit() ->
     config:get_integer("couch_views", "value_size_limit", ?VALUE_SIZE_LIMIT).
+
+
+update_tasks(NumChanges, false) ->
+    [Changes] = couch_task_status:get([changes_done]),
+    Changes2 = Changes + NumChanges,
+    couch_task_status:update([{changes_done, Changes2}]);
+
+update_tasks(NumChanges, LastSeq) ->
+    [Changes] = couch_task_status:get([changes_done]),
+    Changes2 = Changes + NumChanges,
+    VS = fabric2_fdb:seq_to_vs(LastSeq),
+    VS1 = convert_version_stamp(VS),
+    couch_task_status:update([{changes_done, Changes2},
+        {current_version_stamp, VS1}]).
+
+
+convert_version_stamp({_, Stamp, _, DocNumber}) ->
+    VS = integer_to_list(Stamp) ++ "-" ++ integer_to_list(DocNumber),
+    list_to_binary(VS);
+
+convert_version_stamp(_) ->

Review comment:
       What would cause us to reach this state?

##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -231,10 +231,24 @@ maybe_set_build_status(TxDb, Mrst1, _ViewVS, State) ->
 get_update_start_state(TxDb, Mrst, #{db_seq := undefined} = State) ->
     ViewVS = couch_views_fdb:get_creation_vs(TxDb, Mrst),
     ViewSeq = couch_views_fdb:get_update_seq(TxDb, Mrst),
+    DbSeq = fabric2_db:get_update_seq(TxDb),
+    DbVS = fabric2_fdb:seq_to_vs(DbSeq),
+    ViewVS1 = convert_version_stamp(ViewVS),
+    DbVS1 = convert_version_stamp(DbVS),
+
+    couch_task_status:add_task([
+        {type, indexer},
+        {database, Mrst#mrst.db_name},
+        {design_document, Mrst#mrst.idx_name},
+        {changes_done, 0},

Review comment:
       Will changes always be 0? What happens if an index is half-built and the indexer is starting up again?

##########
File path: src/couch_views/src/couch_views_indexer.erl
##########
@@ -368,6 +382,7 @@ write_docs(TxDb, Mrst, Docs, State) ->
     if LastSeq == false -> ok; true ->
         couch_views_fdb:set_update_seq(TxDb, Sig, LastSeq)
     end,
+    update_tasks(length(Docs), LastSeq),

Review comment:
       I think you can use `DocsNumber` since that is the count of the number of docs processed




----------------------------------------------------------------
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