Review and document concurrency semantics #19

New Issue

roysc · 2023-10-05T14:08:43Z

roysc commented

2023-10-05 14:08:43 +00:00

The semantics of indexer concurrency and the constraints on its usage should be well-defined and documented.

Indexer transaction (Batch) objects are currently not thread-safe, so all writes are synchronized with mutexes. If this has a performance impact, we could

guarantee Batch can be used concurrently and remove redundant locks, or
refactor so a Tx object of some kind can be used for each thread

The semantics of indexer concurrency and the constraints on its usage should be well-defined and documented. Indexer transaction (`Batch`) objects are currently not thread-safe, so all writes are synchronized with mutexes. If this has a performance impact, we could 1. guarantee `Batch` can be used concurrently and remove redundant locks, or 2. refactor so a `Tx` object of some kind can be used for each thread

telackey commented

2023-10-05 18:22:51 +00:00

What is the performance impact of the current locking scheme? As long as the current scheme is enforcing correct behavior, I would be hesitant to replace it unless it was demonstrated to be an actual bottleneck.

I would probably begin by improving logging around lock contention, showing the count of how many threads/routines were waiting for the lock, how long they had to wait, etc.

That work would be needed anyway to demonstrate any improvement offered by a new scheme; or it may show that this is not currently a bottleneck.

What is the performance impact of the current locking scheme? As long as the current scheme is enforcing correct behavior, I would be hesitant to replace it unless it was demonstrated to be an actual bottleneck. I would probably begin by improving logging around lock contention, showing the count of how many threads/routines were waiting for the lock, how long they had to wait, etc. That work would be needed anyway to demonstrate any improvement offered by a new scheme; or it may show that this is not currently a bottleneck.

roysc commented

2023-10-05 18:35:07 +00:00

Good point, we can repurpose this as a "review" issue.

roysc changed title from ~~Improve concurrency semantics~~ to Review concurrency semantics

2023-10-05 18:35:56 +00:00

roysc changed title from ~~Review concurrency semantics~~ to Review and document concurrency semantics

2023-10-05 18:44:32 +00:00

telackey added the

Kind/Enhancement

Priority

Low

labels 2023-10-05 22:01:28 +00:00