diff options
| author | Nick Vatamaniuc <vatamane@apache.org> | 2018-09-19 15:56:55 -0400 |
|---|---|---|
| committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2018-09-19 17:51:48 -0400 |
| commit | 52ed7fbf48b953c3cd0dd5c058186a6af01dbc64 (patch) | |
| tree | 0270b34b3a6bcb55f0810b7013d240b8614d8967 | |
| parent | b4bfc529efdba4b971cd8351dc1fa6b552089744 (diff) | |
| download | couchdb-52ed7fbf48b953c3cd0dd5c058186a6af01dbc64.tar.gz | |
Ignore local nodes in read repair filtering
Previosly local node revisions were causing `badmatch` failures in read repair
filter. Node sequences already filtered out local nodes while NodeRevs didn't, so
during matching `{Node, NodeSeq} = lists:keyfind(Node, 1, NodeSeqs)` Node would
not be found in the list and crash.
Example of crash:
```
fabric_rpc:update_docs/3 error:{badmatch,false}
[{fabric_rpc,'-read_repair_filter/3-fun-1-',4,[{file,"src/fabric_rpc.erl"},{line,360}]},
```
| -rw-r--r-- | src/fabric/src/fabric_rpc.erl | 5 | ||||
| -rw-r--r-- | src/fabric/test/fabric_rpc_purge_tests.erl | 24 |
2 files changed, 26 insertions, 3 deletions
diff --git a/src/fabric/src/fabric_rpc.erl b/src/fabric/src/fabric_rpc.erl index c68422969..11e675464 100644 --- a/src/fabric/src/fabric_rpc.erl +++ b/src/fabric/src/fabric_rpc.erl @@ -347,7 +347,8 @@ read_repair_filter(DbName, Docs, NodeRevs, Options) -> % as the read_repair option to update_docs. read_repair_filter(Db, Docs, NodeRevs) -> [#doc{id = DocId} | _] = Docs, - Nodes = lists:usort([Node || {Node, _} <- NodeRevs, Node /= node()]), + NonLocalNodeRevs = [NR || {N, _} = NR <- NodeRevs, N /= node()], + Nodes = lists:usort([Node || {Node, _} <- NonLocalNodeRevs]), NodeSeqs = get_node_seqs(Db, Nodes), DbPSeq = couch_db:get_purge_seq(Db), @@ -360,7 +361,7 @@ read_repair_filter(Db, Docs, NodeRevs) -> {Node, NodeSeq} = lists:keyfind(Node, 1, NodeSeqs), NodeSeq >= DbPSeq - Lag end, - RecentNodeRevs = lists:filter(NodeFiltFun, NodeRevs), + RecentNodeRevs = lists:filter(NodeFiltFun, NonLocalNodeRevs), % For each node we scan the purge infos to filter out any % revisions that have been locally purged since we last diff --git a/src/fabric/test/fabric_rpc_purge_tests.erl b/src/fabric/test/fabric_rpc_purge_tests.erl index 26507cf0b..4eafb2bc4 100644 --- a/src/fabric/test/fabric_rpc_purge_tests.erl +++ b/src/fabric/test/fabric_rpc_purge_tests.erl @@ -46,6 +46,7 @@ main_test_() -> lists:map(fun wrap/1, [ ?TDEF(t_filter), ?TDEF(t_filter_unknown_node), + ?TDEF(t_filter_local_node), ?TDEF(t_no_filter_old_node), ?TDEF(t_no_filter_different_node), ?TDEF(t_no_filter_after_repl) @@ -58,6 +59,7 @@ main_test_() -> lists:map(fun wrap/1, [ ?TDEF(t_filter), ?TDEF(t_filter_unknown_node), + ?TDEF(t_filter_local_node), ?TDEF(t_no_filter_old_node), ?TDEF(t_no_filter_different_node), ?TDEF(t_no_filter_after_repl) @@ -172,6 +174,26 @@ t_no_filter_different_node({DbName, DocId, OldDoc, PSeq}) -> ?assertEqual({ok, OldDoc}, open_doc(DbName, DocId)). +t_filter_local_node({DbName, DocId, OldDoc, PSeq}) -> + ?assertEqual({not_found, missing}, open_doc(DbName, DocId)), + create_purge_checkpoint(DbName, PSeq), + + % Create a valid purge for a different node + TgtNode = list_to_binary(atom_to_list('notfoo@127.0.0.1')), + create_purge_checkpoint(DbName, 0, TgtNode), + + % Add a local node rev to the list of node revs. It should + % be filtered out + {Pos, [Rev | _]} = OldDoc#doc.revs, + RROpts = [{read_repair, [ + {tgt_node(), [{Pos, Rev}]}, + {node(), [{1, <<"123">>}]} + ]}], + rpc_update_doc(DbName, OldDoc, RROpts), + + ?assertEqual({ok, OldDoc}, open_doc(DbName, DocId)). + + t_no_filter_after_repl({DbName, DocId, OldDoc, PSeq}) -> ?assertEqual({not_found, missing}, open_doc(DbName, DocId)), create_purge_checkpoint(DbName, PSeq), @@ -282,4 +304,4 @@ tgt_node() -> tgt_node_bin() -> - iolist_to_binary(atom_to_list(tgt_node())).
\ No newline at end of file + iolist_to_binary(atom_to_list(tgt_node())). |