Permalink
Browse files

Fix indexing of multiple values for the same Key/DocID pair

Some places in the code, like the compactor and cleaner, expected
the first element of each value tuple to be the partition ID. This
was not the case for when the map function emits multiple values
for the same key.
This change makes sure the first element of the value tuple is
always a partition ID.

Issue observed in MB-4760 and MB-4774.

Change-Id: I21187023e3b20fa77530c60caa911bfb3b0bdc1f
Reviewed-on: http://review.couchbase.org/13067
Tested-by: Filipe David Borba Manana <fdmanana@gmail.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Damien Katz <damien@couchbase.com>
  • Loading branch information...
1 parent 08e7f87 commit 6319846fa68c73580e5ead96dbe27868447f730f @fdmanana fdmanana committed with fdmanana Feb 7, 2012
@@ -44,7 +44,8 @@ test_files = \
test/10-updates-cleanup.t \
test/11-updates-cleanup-many-views.t \
test/12-errors.t \
- test/13-progressive-cleanup.t
+ test/13-progressive-cleanup.t \
+ test/14-duplicated-keys-per-doc.t
compiled_files = \
ebin/couch_set_view.app \
@@ -325,9 +325,7 @@ do_fold_reduce(Group, View, Fun, Acc, Options0) ->
Options0;
_ ->
ExcludeBitmask = ?set_pbitmask(Group) bor ?set_cbitmask(Group),
- FilterFun = fun(value, {_K, {dups, [{PartId, _} | _]}}) ->
- ((1 bsl PartId) band ?set_abitmask(Group)) =/= 0;
- (value, {_K, {PartId, _}}) ->
+ FilterFun = fun(value, {_K, {PartId, _}}) ->
((1 bsl PartId) band ?set_abitmask(Group)) =/= 0;
(branch, {_, _, PartsBitmap}) ->
case PartsBitmap band ExcludeBitmask of
@@ -436,8 +434,8 @@ reduce_to_count(Reductions) ->
couch_btree:final_reduce(
fun(reduce, KVs) ->
Count = lists:sum(
- [case V of {dups, Vals} -> length(Vals); _ -> 1 end
- || {_,V} <- KVs]),
+ [case V of {_PartId, {dups, Vals}} -> length(Vals); _ -> 1 end
+ || {_, V} <- KVs]),
{Count, [], 0};
(rereduce, Reds) ->
Count = lists:foldl(fun({C, _, _}, Acc) -> Acc + C end, 0, Reds),
@@ -92,12 +92,7 @@ compact_group(Group, EmptyGroup, SetName, FileName, CompactFileName) ->
end,
FilterFun = fun({_Key, {PartId, _}}) ->
- try
- ((1 bsl PartId) band ?set_cbitmask(Group)) =:= 0
- catch Tag:Error ->
- % TODO remove this try catch expression - left for debugging MB-4732
- erlang:error({compact_filter_error, {Tag, Error}, {part_id, PartId}, {cbitmask, ?set_cbitmask(Group)}})
- end
+ ((1 bsl PartId) band ?set_cbitmask(Group)) =:= 0
end,
% First copy the id btree.
{ok, NewIdBtreeRoot, Acc1} = couch_btree_copy:copy(
@@ -608,10 +608,10 @@ view_insert_doc_query_results(DocId, PartitionId, [ResultKVs | RestResults],
{NewKVs, NewViewIdKeysAcc} = lists:foldl(
fun({Key, Val}, {[{{Key, _DocId} = Kd, PrevVal} | AccRest], AccVid}) ->
AccKv2 = case PrevVal of
- {dups, Dups} ->
- [{Kd, {dups, [{PartitionId, Val} | Dups]}} | AccRest];
- _ ->
- [{Kd, {dups, [{PartitionId, Val}, PrevVal]}} | AccRest]
+ {PartitionId, {dups, Dups}} ->
+ [{Kd, {PartitionId, {dups, [Val | Dups]}}} | AccRest];
+ {PartitionId, UserPrevVal} ->
+ [{Kd, {PartitionId, {dups, [Val, UserPrevVal]}}} | AccRest]
end,
{AccKv2, [{View#set_view.id_num, Key} | AccVid]};
({Key, Val}, {AccKv, AccVid}) ->
@@ -32,21 +32,21 @@ detuple_kvs([KV | Rest], Acc) ->
expand_dups([], Acc) ->
lists:reverse(Acc);
-expand_dups([{Key, {dups, Vals}} | Rest], Acc) ->
- Expanded = lists:map(fun({PartId, Val}) -> {Key, {PartId, Val}} end, Vals),
+expand_dups([{Key, {PartId, {dups, Vals}}} | Rest], Acc) ->
+ Expanded = lists:map(fun(Val) -> {Key, {PartId, Val}} end, Vals),
expand_dups(Rest, Expanded ++ Acc);
expand_dups([{_Key, {_PartId, _Val}} = Kv | Rest], Acc) ->
expand_dups(Rest, [Kv | Acc]).
expand_dups([], _Abitmask, Acc) ->
lists:reverse(Acc);
-expand_dups([{Key, {dups, [{PartId, _} | _] = Vals}} | Rest], Abitmask, Acc) ->
+expand_dups([{Key, {PartId, {dups, Vals}}} | Rest], Abitmask, Acc) ->
case (1 bsl PartId) band Abitmask of
0 ->
expand_dups(Rest, Abitmask, Acc);
_ ->
- Expanded = lists:map(fun({_PartId, _Val} = V) -> {Key, V} end, Vals),
+ Expanded = lists:map(fun(V) -> {Key, {PartId, V}} end, Vals),
expand_dups(Rest, Abitmask, Expanded ++ Acc)
end;
expand_dups([{_Key, {PartId, _Val}} = Kv | Rest], Abitmask, Acc) ->
@@ -60,8 +60,6 @@ expand_dups([{_Key, {PartId, _Val}} = Kv | Rest], Abitmask, Acc) ->
partitions_map([], BitMap) ->
BitMap;
-partitions_map([{_Key, {dups, [{PartitionId, _Val} | _]}} | RestKvs], BitMap) ->
- partitions_map(RestKvs, BitMap bor (1 bsl PartitionId));
partitions_map([{_Key, {PartitionId, _Val}} | RestKvs], BitMap) ->
partitions_map(RestKvs, BitMap bor (1 bsl PartitionId)).
Oops, something went wrong. Retry.

0 comments on commit 6319846

Please sign in to comment.