Count async MR results against the sink buffer size cap #529

Merged
merged 2 commits into from Apr 15, 2013

Conversation

Projects
None yet
2 participants
@beerriot
Contributor

beerriot commented Apr 9, 2013

While chasing another issue, I found that MR results delivered to an FSM sink via gen_fsm:send_event/2 are not counted against the buffer size limit. This method is used to deliver 10 results before gen_fsm:sync_send_event/3 is used once. Since only the results sent synchronously were counted against the buffer size limit, the limit actually imposed was effectively 10 times higher than expected.

The first commit in this PR modifies the eunit test to demonstrate the issue. The second commit fixes the problem.

Bryan Fink added some commits Apr 9, 2013

Bryan Fink
failing test: async results do not decrement buffer counter
This test demonstrates that all results delivered by gen_fsm:send_event
are not counted against the buffer limit. This means that MR pipes using
FSM sink type are effectively using a buffer 10x the size they think
they are, because gen_fsm:*sync_*send_event is only used once every 10
results.
Bryan Fink
count async MR results against the sink buffer size cap
This fixes the problem demonstrated by the test in the previous commit.

@ghost ghost assigned beerriot and engelsanchez Apr 9, 2013

@engelsanchez

This comment has been minimized.

Show comment Hide comment
@engelsanchez

engelsanchez Apr 12, 2013

Contributor

I studied the code and it looks like an obvious omission. Also verified the eunit test fails when data is sent asynchronously to the mrc sink, but passes again after the changes are applied. Other eunit tests are failing but seem unrelated.

👍 💃 ⛵️ !

Contributor

engelsanchez commented Apr 12, 2013

I studied the code and it looks like an obvious omission. Also verified the eunit test fails when data is sent asynchronously to the mrc sink, but passes again after the changes are applied. Other eunit tests are failing but seem unrelated.

👍 💃 ⛵️ !

@ghost ghost assigned beerriot Apr 15, 2013

@beerriot beerriot merged commit 95c5a2a into master Apr 15, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment