[warning] Unexpected event in 'connecting' sql
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 42/100
Research direction
Start by tracing the ejabberd_sql p1_fsm connecting callback and the State#state.pending_requests flow shown in the issue. Reproduce or inspect the warning event, then verify that requests encountered while replaying after a SQL connection failure are queued again without losing or duplicating them.
Written by the indexing model from the issue text.
Description
ejabberd_sql mod uses the p1_fsm behaviour to queue & play the queries which are not able run due to any sql idle state or down of connection, ejabberd has the default code to handle queuing queries and executing the queued queries when connection get established again,
but when executing the queued queries, if in case of sql state goes down again due to any reason.. p1_fsm behaviour should re-queue the queries which were already queued once when the connection was idle, thus it is not happening in the current code base.
So, when the connecting callback getting called by the p1_fsm on re-connecting.. the State#state.pending_requests should be enqueued again
here is the warning log which I've got:
[warning] Unexpected event in 'connecting':
{sql_cmd, -576404747677, #Ref<0.560196108.4265934851.116676>},
[
alias,
{<0.8175.3>, <<"919512282008">>, <<"';">>}
],
[
<<"select contact_permission from chat_users where username='">>
],
{sql_query, ...}
here is the code that handled the warning log
connecting(Event, State) ->
?WARNING_MSG("Unexpected event in 'connecting': ~p",
[Event]),
{next_state, connecting, State}.
thus by seeing the code, I could effectively tell that this is being explicitly handled to go for the next state without re-queuing it, but since the log shows the pattern which was casted by p1_fsm callback, so I've added the below followed up functions to enqueue the queries again in the state
connecting({{sql_cmd, _ReqId, _Ref}, From, Query, Meta} = Req, State) ->
?DEBUG("Re-queuing expanded async request while connecting:n\tp", [Req]),
Command = {sql_query, Query, Meta},
Timestamp = erlang:monotonic_time(millisecond),
PendingRequests =
enqueue_pending({sql_cmd, Command, From, Timestamp}, State),
{next_state, connecting,
State#state{pending_requests = PendingRequests}};
connecting({sql_cmd, Command, From, Timestamp} = Req, State) ->
?DEBUG("Re-queuing async request while connecting:n\tp", [Req]),
PendingRequests =
enqueue_pending({sql_cmd, Command, From, Timestamp}, State),
{next_state, connecting,
State#state{pending_requests = PendingRequests}};
enqueue_pending(Item, State) ->
try p1_queue:in(Item, State#state.pending_requests)
catch error:full ->
Err = <<"SQL request queue is overfilled">>,
?ERROR_MSG("~ts, bouncing all pending requests", [Err]),
Q = p1_queue:dropwhile(
fun({sql_cmd, _, To, TS}) ->
reply(To, {error, Err}, TS),
true
end, State#state.pending_requests),
p1_queue:in(Item, Q)
end.
yet I'm not so ok with what I've done to safely the handled the rare case of sql idle/down state, I'm anticipating ejabberd community folks to take a look at the flow and the code changes I've done here to resolve this issue, and let me know is there any other conditional check should I've to add.
- Dominant language
- Erlang
- Stars
- 6.7k
- Forks
- 1.6k
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from processone/ejabberd
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
processone/ejabberd#4597 ·
-
API permissions Open
processone/ejabberd#4619 · 3 comments · 1 assignee ·
-
Difficulty 3/5 1-2 days Newbie friendliness 55/100
processone/ejabberd#4616 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 55/100
processone/ejabberd#4614 · 1 comment ·
-
Difficulty 4/5 3-5 days Newbie friendliness 55/100
processone/ejabberd#4613 · 1 comment ·
All issues in processone/ejabberd
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
code-quality refactoring
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
github/gh-aw-firewall#8816 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
getgrav/grav-plugin-api#45 ·
-
integration:quickjs org:external priority:backlog topic:code-interpreter topic:middleware type:feature
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
langchain-ai/deepagents#6450 ·