apache / apache/arrow

Recursive locking in RestartTask makes it impossible to avoid context switches

Open
#48,332 1 comment 0 reactions 0 assignees View on GitHub
Component: C++ Type: enhancement
Dominant language
C++
Stars
17.1k
Forks
4.3k
Avg merge
3d 13h
Merged PRs (30d)
88

Description

### Describe the enhancement requested

Our C++ application is using fibers for concurrency inside its `RandomAccessFile` implementation.
`arrow::*Reader` classes switch contexts and blocks threads interacting with them.
This causes thread starvation and a dead-lock inside our `RandomAccessFile` implementation, as it needs free threads to finish the work started by `ReadNext`.
I have tried to completely avoid hidden threads and context switches here, by implementing a direct executor:

```
static struct
: public arrow::internal::Executor
{
int GetCapacity() override { return 1; };

arrow::Status SpawnReal(
arrow::internal::TaskHints,
arrow::internal::FnOnce task,
arrow::StopToken,
StopCallback&&) override
{
std::move(task)();
return arrow::Status::OK();
}
} DirectExecutor;
```

Unfortunately, this also blocks, because of the recursive lock happening here:
https://github.com/apache/arrow/blob/apache-arrow-22.0.0/cpp/src/arrow/util/async_generator.h#L1708
https://github.com/apache/arrow/blob/apache-arrow-22.0.0/cpp/src/arrow/util/async_generator.h#L1776

Is it possible to rewrite `DoRestartTask` in a way, so it would release the lock and if `!spawn_status.ok()`, retake it back?

The blocking is still possible to avoid by using the Async API, but not the context switches.

### Component(s)

C++

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.