Do not fail complete reindex when one object fails

There are cases when we do a reindex where the object name in the serialized JSON does not match the name we get when selecting `(id, name)` from the database. Currently when this lookup fails, the entire reindex crashes. This change makes it so that if we fail to find the name, instead of failing entirely, we output a list of objects that failed to reindex to the user. Signed-off-by: Nathan L Smith <[email protected]>
chef · Oct 11, 2016 · 1d68f9a · 1d68f9a
1 parent 121d920
commit 1d68f9a
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 25 deletions.
diff --git a/src/oc_erchef/apps/oc_chef_wm/src/chef_reindex.erl b/src/oc_erchef/apps/oc_chef_wm/src/chef_reindex.erl
@@ -44,11 +44,11 @@
 %% reindexing.  Does not drop existing index entries beforehand; this
 %% is explicitly a separate step.
 -spec reindex(Ctx :: chef_db:db_context(),
-              OrgInfo :: org_info()) -> ok.
+              OrgInfo :: org_info()) -> {ok, list()}.
 reindex(Ctx, {OrgId, _OrgName}=OrgInfo) ->
     AllIndexes = fetch_org_indexes(Ctx, OrgId),
-    [ reindex(Ctx, OrgInfo, Index) || Index <- AllIndexes ],
-    ok.
+    Missing = [ reindex(Ctx, OrgInfo, Index) || Index <- AllIndexes ],
+    {ok, lists:flatten(Missing)}.
 
 fetch_org_indexes(Ctx, OrgId) ->
     BuiltInIndexes = [node, role, environment, client],
@@ -103,7 +103,7 @@ fetch_org_indexes(Ctx, OrgId) ->
 %% the given indexable type.
 -spec reindex(DbContext :: chef_db:db_context(),
               OrgInfo :: org_info(),
-              Index :: index()) -> ok.
+              Index :: index()) -> list().
 reindex(Ctx, {OrgId, _OrgName}=OrgInfo, Index) ->
     NameIdDict = chef_db:create_name_id_dict(Ctx, Index, OrgId),
     %% Grab all the database IDs to do batch retrieval on
@@ -115,7 +115,7 @@ reindex(Ctx, {OrgId, _OrgName}=OrgInfo, Index) ->
 -spec reindex_by_id(DbContext :: chef_db:db_context(),
                     OrgInfo :: org_info(),
                     Index :: index(),
-                    Ids :: [<<_:256>>]) -> ok.
+                    Ids :: [<<_:256>>]) -> list().
 reindex_by_id(Ctx, {OrgId, _OrgName} = OrgInfo, Index, Ids) ->
     %% This is overkill, but workable until we have more robust reindexing
     NameIdDict = chef_db:create_name_id_dict(Ctx, Index, OrgId),
@@ -125,7 +125,7 @@ reindex_by_id(Ctx, {OrgId, _OrgName} = OrgInfo, Index, Ids) ->
 -spec reindex_by_name(DbContext :: chef_db:db_context(),
                       OrgInfo :: org_info(),
                       Index :: index(),
-                      Names :: [binary()]) -> ok.
+                      Names :: [binary()]) -> list().
 reindex_by_name(Ctx, {OrgId, _OrgName} = OrgInfo, Index, Names) ->
     NameIdDict = chef_db:create_name_id_dict(Ctx, Index, OrgId),
     Ids = lists:foldl(
@@ -174,25 +174,23 @@ decompress_and_decode(Object) ->
                     BatchSize :: non_neg_integer(),
                     OrgInfo :: org_info(),
                     Index :: index(),
-                    NameIdDict :: dict()) -> ok.
+                    NameIdDict :: dict()) -> list().
 batch_reindex(Ctx, Ids, BatchSize, OrgInfo, Index, NameIdDict) when is_list(Ids) ->
-    DoBatch = fun(Batch, _Acc) ->
-                      ok = index_a_batch(Ctx, Batch, OrgInfo, Index, NameIdDict),
-                      ok
+    DoBatch = fun(Batch, MissingList) ->
+                      [index_a_batch(Ctx, Batch, OrgInfo, Index, NameIdDict) | MissingList]
                  end,
-    chefp:batch_fold(DoBatch, Ids, ok, BatchSize).
+    chefp:batch_fold(DoBatch, Ids, [], BatchSize).
 
 %% @doc Helper function to retrieve and index objects for a single
 %% batch of database IDs.
 -spec index_a_batch(chef_db:db_context(),
                     BatchOfIds :: [object_id()],
                     OrgInfo :: org_info(),
                     Index :: index(),
-                    NameIdDict :: dict()) -> ok.
+                    NameIdDict :: dict()) -> list().
 index_a_batch(Ctx, BatchOfIds, {OrgId, OrgName}, Index, NameIdDict) ->
     SerializedObjects = chef_db:bulk_get(Ctx, OrgName, chef_object_type(Index), BatchOfIds),
-    ok = send_to_index_queue(OrgId, Index, SerializedObjects, NameIdDict),
-    ok.
+    send_to_index_queue(OrgId, Index, SerializedObjects, NameIdDict, []).
 
 %% @doc Resolve an index to the name of the corresponding Chef object type.
 chef_object_type(Index) when is_binary(Index) -> data_bag_item;
@@ -204,10 +202,11 @@ chef_object_type(Index)                       -> Index.
 -spec send_to_index_queue(OrgId :: object_id(),
                           Index :: index(),
                           SerializedObjects :: [binary()] | [ej:json_object()],
-                          NameIdDict :: dict()) -> ok.
-send_to_index_queue(_, _, [], _) ->
-    ok;
-send_to_index_queue(OrgId, Index, [SO|Rest], NameIdDict) ->
+                          NameIdDict :: dict(),
+                          MissingList :: list()) -> list().
+send_to_index_queue(_, _, [], _, MissingList) ->
+    MissingList;
+send_to_index_queue(OrgId, Index, [SO|Rest], NameIdDict, MissingList) ->
     %% All object types are returned from chef_db:bulk_get/4 as
     %% binaries (compressed or not) EXCEPT for clients, which are
     %% returned as EJson directly, because none of their
@@ -216,12 +215,18 @@ send_to_index_queue(OrgId, Index, [SO|Rest], NameIdDict) ->
     PreliminaryEJson = decompress_and_decode(SO),
     NameKey = name_key(chef_object_type(Index)),
     ItemName = ej:get({NameKey}, PreliminaryEJson),
-    {ok, ObjectId} = dict:find(ItemName, NameIdDict),
-    StubRec = stub_record(Index, OrgId, ObjectId, ItemName, PreliminaryEJson),
-    %% Since we get here via valid `Index', we don't have to check that the objects are
-    %% indexable.
-    ok = oc_chef_object_db:add_to_solr_async(StubRec, PreliminaryEJson),
-    send_to_index_queue(OrgId, Index, Rest, NameIdDict).
+    NewMissingList = case dict:find(ItemName, NameIdDict) of
+        {ok, ObjectId} ->
+            StubRec = stub_record(Index, OrgId, ObjectId, ItemName, PreliminaryEJson),
+            %% Since we get here via valid `Index', we don't have to check that the objects are
+            %% indexable.
+            oc_chef_object_db:add_to_solr_async(StubRec, PreliminaryEJson),
+            MissingList;
+        error ->
+            lager:warning("skipping: no id found for name ~p", [ItemName]),
+            [{Index, ItemName} | MissingList]
+    end,
+    send_to_index_queue(OrgId, Index, Rest, NameIdDict, NewMissingList).
 
 %% @doc Determine the proper key to use to retrieve the unique name of
 %% an object from its EJson representation.

diff --git a/src/oc_erchef/priv/reindex-opc-organization b/src/oc_erchef/priv/reindex-opc-organization
@@ -98,7 +98,15 @@ perform(drop, _Context, {OrgId, OrgName}) ->
     ok = rpc:call(?ERCHEF, chef_index, delete_search_db, [OrgId]);
 perform(reindex, Context, {_OrgId, OrgName}=OrgInfo) ->
     io:format("Sending all data for organization '~s' to be indexed again.  It may take some time before everything is available via search.~n", [OrgName]),
-    ok = rpc:call(?ERCHEF, chef_reindex, reindex, [Context, OrgInfo]);
+    {ok, Missing} = rpc:call(?ERCHEF, chef_reindex, reindex, [Context, OrgInfo]),
+    case Missing of
+        [] ->
+            ok;
+        _ ->
+            io:format(standard_error, "~nThe following objects in organization \"~s\" were unable to be reindexed:~n~n", [OrgName]),
+            [io:format(standard_error, "\t~s ~s~n", [Type, Name]) || {Type, Name} <- Missing],
+            io:format(standard_error, "~n", [])
+    end;
 perform(complete, Context, OrgInfo) ->
     %% Just do everything
     perform(drop, Context, OrgInfo),