Bug #54
Multiple multi-reads (of size 1, typically) do not return the correct number of events.
0%
Description
For a multi-read of size 1, a client should receive 2 events (GDP_EVENT_DATA followed by GDP_EVENT_EOS). In case of N multi-reads of size 1, this translates to 2*N events. But this doesn't always happen. In as small as 2 multi-reads, the number of actual events range from 2 to 4 across different runs.
Attached is a sample Python program to reproduce it; it attempts to implement an asynchronous read by calling multi-reads of size 1. The same behavior is seen in C as well.
The server returns the correct number of events (verified with WireShark). The bug is in the client library processing. My hunch is the bug lies is in the processing of the PDU starting at source:gdp/gdp_main.c#L425.
In source:gdp/gdp_priv.h#L409, the assumption is that a request can not have a WAITING
status, while maintaining an active subscription (or multi-read, for that matter). The logic in source:gdp/gdp_main.c#L425 follows this assumption. However with quite a few multi-reads of size 1, this assumption fails: a reader may be WAITING
on _gdp_invoke
of a new multi-read command when a PDU as a result of a previous multi-read arrives.
Attached is a debug log for 2 multi-reads of size 1, which should see 4 events but only 2 events are passed back to the client. See line#150, where req->state
is WAITING
; the PDU that was received is the data from result of multi-read and the control flow goes to the if
block at source:gdp/gdp_main.c#L425 rather than to source:gdp/gdp_main.c#L443
History
#1 Updated by Nitesh Mor over 6 years ago
commit:4d6ff457 seems to have it fixed. I can no longer reproduce the bug.
Leaving this open though, just in case you have more sub-issues to be addressed that I might not be aware of. Feel free to close it.
#2 Updated by Eric Allman over 6 years ago
- Status changed from New to Feedback
Should have been fixed in commit:4d6ff457. However, it turns out that small multi-reads still serialize behind the actual read command, so commit:8204d4b2 adds gdp_gcl_read_async
, which asynchronously returns exactly one event PDU for each read.
#3 Updated by Nitesh Mor over 6 years ago
- Status changed from Feedback to Closed
Verified read_async works. Closing this. Thanks