ipfs / ipfs/go-graphsync

Remove or expand buffer limits on RequestManager and ResponseManager queues

Open
#249 2 comments 0 reactions 0 assignees View on GitHub
effort/hours exp/intermediate need/analysis P3
Dominant language
Go
Stars
103
Forks
41
PR merge metrics
No merged PRs in 30d

Description

# What

The basic architecture of the request and response manager is as follows: all public methods put messages into a buffered channel that is processed sequentially by an internal thread. (see https://github.com/ipfs/go-graphsync/blob/main/docs/architecture.md#actor-pattern-in-requestmanager-and-responsemanager for explanation)

We've now seen multiple deadlocks from these channels reaching their buffer limits. Moreover, the problem is usually that actual message processing of the internal thread indirectly gets stuck sending more messages to the queue.

I think we should remove limits on these queues for now. We can use use well tread architectures for a single consumer thread-safe queues using sync.Mutex and sync.Cond.Wait -- see https://www.justsoftwaresolutions.co.uk/threading/implementing-a-thread-safe-queue-using-condition-variables.html, https://dev.to/narasimha1997/crafting-a-concurrent-queue-in-golang-2n, and the notifications system in this repo: https://github.com/ipfs/go-graphsync/blob/main/notifications/publisher.go#L214-L235

If we need to set an upper limit, it's not hard to add.

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.