Clever / optimus

Concurrently extract, transform, and load tables of data in Go
Apache License 2.0
34 stars 7 forks source link

test more transform error cases #21

Closed azylman closed 9 years ago

azylman commented 9 years ago

I found a bug where if you had two transformedTables chained together and the second one errored, it would cause deadlock because it would call the Stop method on the first one and then the first one would never close its output rows, causing the second one to wait forever on the first since we drain the previous table before ending.

I improved the error test case so that it tests i good transforms, a bad transform, and j good transforms for everything from (0, 5) to (5, 0). This reproduced the bug, and then I fixed it (it actually only needed two transforms to repro, e.g. two Eaches, but I figured this was even better).

Longer-term, I'd like to find a way to make the transformedTable table end right away and not wait for previous tables, while still signaling previous tables to stop, but that seems harder to synchronize. You'd need to make sure the goroutine writing to its output is signaled to stop and stops before you close its rows so you don't try and write to a closed channel. It also only has real benefits in the case of a misbehaving source, where it doesn't properly obey the Stop command.

azylman commented 9 years ago

@kvigen curious for your thoughts on the right solution here, and also assigned this to you

kvigen commented 9 years ago

interesting, lgtm! Don't have good thoughts on the long-term idea off the top of my head.