Couch index fixes #4491

rnewson · 2023-03-22T14:34:22Z

couch_index_server can crash if;

an index, opened with new_index, crashes while doing so
the db event listener pid crashes

This PR improves behaviour as follows;

log original stack in couch_event_listener_mfa:db_event (but still throw)
prevent couch_index_server:handle_db_event from crashing
track the opener pids in couch_index_server, don't crash couch_index_server if they terminate abnormally

src/couch_index/src/couch_index_server.erl

This reverts commit 937ccb6.

src/couch_index/src/couch_index_server.erl

nickva · 2023-03-22T19:12:38Z

src/couch_index/src/couch_index_server.erl

+ fun(DbShard) ->
+ lists:foreach(
+ fun({_DbShard, {_DDocId, Sig}}) ->
+ % check if there are other ddocs with the same Sig for the same db
+ SigDDocs = ets:match_object(St#st.by_db, {DbShard, {'$1', Sig}}),
+ if
+ length(SigDDocs) > 1 ->
+ % remove records from by_db for this DDoc
+ Args = [DbShard, DDocId, Sig],
+ gen_server:cast(St#st.server_name, {rem_from_ets, Args});
+ true ->
+ % single DDoc with this Sig - close couch_index processes
+ case ets:lookup(St#st.by_sig, {DbShard, Sig}) of
+ [{_, IndexPid}] ->
+ (catch gen_server:cast(
+ IndexPid, {ddoc_updated, DDocResult}
+ ));
+ [] ->
+ []
+ end
+ end
+ end,
+ ets:match_object(St#st.by_db, {DbShard, {DDocId, '$1'}})
+ )
+ end,
+ DbShards


To avoid deep nesting a utility function might work better shard_ddoc_updated(DbShard, St)?

this is existing code, I'd rather not alter beyond what's essential.

+1, nesting is a bit deep and a helper would help.

Your diff will be smaller if you don't indent silly goose.

src/couch_index/src/couch_index_server.erl

davisp

Few minor changes, mostly biggest around checking the wrong pids in the openers table.

src/couch_index/src/couch_index_server.erl

davisp · 2023-03-22T20:10:03Z

src/couch_index/src/couch_index_server.erl

+ fun(DbShard) ->
+ lists:foreach(
+ fun({_DbShard, {_DDocId, Sig}}) ->
+ % check if there are other ddocs with the same Sig for the same db
+ SigDDocs = ets:match_object(St#st.by_db, {DbShard, {'$1', Sig}}),
+ if
+ length(SigDDocs) > 1 ->
+ % remove records from by_db for this DDoc
+ Args = [DbShard, DDocId, Sig],
+ gen_server:cast(St#st.server_name, {rem_from_ets, Args});
+ true ->
+ % single DDoc with this Sig - close couch_index processes
+ case ets:lookup(St#st.by_sig, {DbShard, Sig}) of
+ [{_, IndexPid}] ->
+ (catch gen_server:cast(
+ IndexPid, {ddoc_updated, DDocResult}
+ ));
+ [] ->
+ []
+ end
+ end
+ end,
+ ets:match_object(St#st.by_db, {DbShard, {DDocId, '$1'}})
+ )
+ end,
+ DbShards


+1, nesting is a bit deep and a helper would help.

davisp

+1

nickva

A few eunit tests are failing

module 'couch_index_ddoc_updated_tests'
  Check ddoc update actions
    couch_index_ddoc_updated_tests:54: check_all_indexers_exit_on_ddoc_change...*failed*
in function couch_index_server:handle_db_event/3 (src/couch_index_server.erl, line 388)
in call from lists:foreach/2 (lists.erl, line 1342)
in call from couch_index_ddoc_updated_tests:'-check_all_indexers_exit_on_ddoc_change/1-fun-7-'/1 (test/eunit/couch_index_ddoc_updated_tests.erl, line 106)
in call from eunit_test:run_testfun/1 (eunit_test.erl, line 71)
in call from eunit_proc:run_test/1 (eunit_proc.erl, line 531)
in call from eunit_proc:with_timeout/3 (eunit_proc.erl, line 356)
in call from eunit_proc:handle_test/2 (eunit_proc.erl, line 514)
in call from eunit_proc:tests_inorder/3 (eunit_proc.erl, line 456)
**error:{badrecord,st}
  output:<<"">>

Would probably be good to have eunit tests exercising some of the new code. One approach there could be to mock couch_index_server and intercept the async opener logic to force it crash.

nickva · 2023-03-22T22:13:41Z

@rnewson forgot to add unit tests #4491 (review)

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

rnewson requested review from davisp and nickva March 22, 2023 14:34

rnewson assigned janl and unassigned janl Mar 22, 2023

rnewson requested a review from janl March 22, 2023 14:35

rnewson force-pushed the couch_index_fixes branch from c81c5c5 to 84b1ed5 Compare March 22, 2023 14:36

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Outdated Show resolved Hide resolved

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Outdated Show resolved Hide resolved

rnewson force-pushed the couch_index_fixes branch from 84b1ed5 to aca3bc2 Compare March 22, 2023 15:09

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Outdated Show resolved Hide resolved

rnewson added 3 commits March 22, 2023 17:46

Revert "catch and log any error from mem3:local_shards"

65ad03c

This reverts commit 937ccb6.

log the original stack trace if Mod:Func throws

451d4bd

don't crash in handle_db_event

f82d961

rnewson force-pushed the couch_index_fixes branch 3 times, most recently from a4f9267 to 8bf45ad Compare March 22, 2023 18:45

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Outdated Show resolved Hide resolved

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Outdated Show resolved Hide resolved

nickva reviewed Mar 22, 2023

View reviewed changes

src/couch_index/src/couch_index_server.erl Show resolved Hide resolved

rnewson force-pushed the couch_index_fixes branch from 8bf45ad to 2220d6b Compare March 22, 2023 19:56

davisp requested changes Mar 22, 2023

View reviewed changes

davisp approved these changes Mar 22, 2023

View reviewed changes

rnewson force-pushed the couch_index_fixes branch 2 times, most recently from 813cae1 to 4526b17 Compare March 22, 2023 20:58

nickva requested changes Mar 22, 2023

View reviewed changes

track index pids during open and don't crash if they do

224d676

rnewson force-pushed the couch_index_fixes branch from 4526b17 to 224d676 Compare March 22, 2023 21:15

rnewson merged commit 0073e76 into main Mar 22, 2023

rnewson deleted the couch_index_fixes branch March 22, 2023 22:00

nickva added a commit that referenced this pull request Dec 14, 2023

Make sure to reply to couch_index_server clients

103bb9c

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

nickva mentioned this pull request Dec 14, 2023

Make sure to reply to couch_index_server clients #4921

Merged

nickva added a commit that referenced this pull request Dec 14, 2023

Make sure to reply to couch_index_server clients

f6d2fcf

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

nickva added a commit that referenced this pull request Dec 14, 2023

Make sure to reply to couch_index_server clients

585231a

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

nickva added a commit that referenced this pull request Dec 14, 2023

Make sure to reply to couch_index_server clients

8508f44

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

nickva added a commit that referenced this pull request Dec 15, 2023

Make sure to reply to couch_index_server clients

380606d

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

nickva added a commit that referenced this pull request Dec 15, 2023

Make sure to reply to couch_index_server clients

d86847d

Previously we didin't always reply to waiting `get_index` calls as sometimes we clobbered or deleted waiters in by_sig ets table. Ref: #4491

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Couch index fixes #4491

Couch index fixes #4491

rnewson commented Mar 22, 2023 •

edited

Loading

nickva Mar 22, 2023

rnewson Mar 22, 2023

davisp Mar 22, 2023

davisp Mar 22, 2023

davisp left a comment

davisp Mar 22, 2023

davisp left a comment

nickva left a comment •

edited

Loading

nickva commented Mar 22, 2023

Couch index fixes #4491

Couch index fixes #4491

Conversation

rnewson commented Mar 22, 2023 • edited Loading

nickva Mar 22, 2023

Choose a reason for hiding this comment

rnewson Mar 22, 2023

Choose a reason for hiding this comment

davisp Mar 22, 2023

Choose a reason for hiding this comment

davisp Mar 22, 2023

Choose a reason for hiding this comment

davisp left a comment

Choose a reason for hiding this comment

davisp Mar 22, 2023

Choose a reason for hiding this comment

davisp left a comment

Choose a reason for hiding this comment

nickva left a comment • edited Loading

Choose a reason for hiding this comment

nickva commented Mar 22, 2023

rnewson commented Mar 22, 2023 •

edited

Loading

nickva left a comment •

edited

Loading