Better handle errors when something goes wrong writing partitions (#154)
authorJacob Quinn <quinn.jacobd@gmail.com>
Thu, 18 Mar 2021 06:22:40 +0000 (00:22 -0600)
committerGitHub <noreply@github.com>
Thu, 18 Mar 2021 06:22:40 +0000 (00:22 -0600)
commit2cacbe5384d3494bb49d929a8b386514f0111c74
tree60626dfade00de7631678126811e7cc0b4d67efd
parenta8aaf95d2dc6baf9a7e015a6baf98ba6221dcf07
Better handle errors when something goes wrong writing partitions (#154)

* Better handle errors when something goes wrong writing partitions

Follow up to #108. There were actually a few different issues all coming
together in @kjanosz's comment. The first being that `ZipFile.Reader`
doesn't like non-main threads touching its files _at all_. The Arrow.jl
problem there is when processing non-first partitions, we were just
`Threads.@spawn`-ing a task which then went off and sometimes became the
proverbial unheard tree falling in the forest. Like really, no one heard
these tasks and their poor exceptions.

The solution there is to be better shepherds of our spawned tasks; we
introduce a `anyerror` atomic bool that threads can set if they run into
issues and then in the main partition handling loop we'll check that. If
a thread ran into something, we'll log out the thread-specific
exception, then throw a generic writing error. This prevents the
"hanging" behavior people were seeing because the felled threads were
actually causing later tasks to hang on `put!`-ing into the
OrderedChannel.

After addressing this, we have the multithreaded ZipFile issue. With the
new `ntasks` keyword, it seems to make sense to me that if you pass
`ntasks=1`, you're really saying, I don't want any concurrency. So
anywhere we were `Threads.@spawn`ing, we now check if `ntasks == 1` and
if so, do an `@async` instead.

* Fix
src/arraytypes/dictencoding.jl
src/write.jl
test/runtests.jl