nagaboinaramgopal commented on PR #14040:
URL: https://github.com/apache/cloudstack/pull/14040#issuecomment-5572162872
> @nagaboinaramgopal , as you are not referring an issue, can you add a
reporduction scheme/scenario?
No linked issue on this one, it came from following the in-sequence command
path in `AgentAttache`, so here is the scenario, and it reproduces
deterministically as a fail-before / pass-after test.
For a given host, in-sequence commands run one at a time, and
`_currentSequence` holds the sequence that is currently in flight. While it is
non-null, any newly arriving in-sequence command is queued in `_requests`
rather than sent (the `executeInSequence() && _currentSequence != null` branch
in `send(...)`).
Sequence that stalls the queue:
1. Command A is in flight to the host, so `_currentSequence = A`. Commands B
and C arrive meanwhile and queue behind it.
2. A completes, so the answer path calls `sendNext(...)`, which pops B and
calls `send(B)`.
3. `send(B)` throws `AgentUnavailableException`. This is the transient case
the catch block exists for: the agent link is briefly down or reconnecting, or
a socket write fails as the command goes out.
4. Before this change, `sendNext` cancelled B but then still ran
`_currentSequence = B` at the end of the method. B was just cancelled, so no
answer will ever arrive for it.
5. `_currentSequence` now points at a dead sequence. C, and every later
in-sequence command to this host, takes the `_currentSequence != null` path and
waits for B. The queue only unwedges when the outer `send(req, wait)` hits its
full timeout and clears `_currentSequence`, so in the meantime each in-sequence
command to that host eats a timeout.
`AgentAttacheSendNextTest.sendNextAdvancesPastAFailedCommandToTheNextQueued`
pins this: it queues a command whose `send()` fails followed by a good one,
then asserts `sendNext` dispatches the good command and leaves
`_currentSequence` on it.
On the unpatched code the test fails, which is exactly the stall:
java.lang.AssertionError: the next queued command should have been
dispatched expected:<200> but was:<null>
Tests run: 1, Failures: 1
With the change it passes (the queued command is dispatched instead of the
queue parking on the cancelled one):
Tests run: 1, Failures: 0
I could not tie it to a specific cluster incident, since it needs a send
failure at the exact moment the queue is drained, but that is the path and the
test reproduces it deterministically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
