Skip to content

Commit 8ff73e9

Browse files
authored
add LimitedSemaphore (#13642)
* treat Semaphore._waiters as length zero when it is None We access the `._waiters` private attribute of the Python asyncio `Semaphore` class. This was changed in Python 3.10.8 (and other versions) to be initialized to `None` instead of an empty deque. Our existing unconditional length checks failed on the new `None` default. This seems to block syncing. python/cpython#97020 https://github.com/python/cpython/compare/v3.10.7..v3.10.8#diff-0fee1befb15023abc0dad2623effa93a304946796929f6cb445d11a57821e737 Reported traceback: ```python-traceback 2022-10-12T20:03:59.367 full_node full_node_server : INFO Connected with full_node {'host': '65.34.144.6', 'port': 8444} 2022-10-12T20:03:59.370 full_node full_node_server : ERROR Exception: object of type 'NoneType' has no len(), {'host': '65.34.144.6', 'port': 8444}. Traceback (most recent call last): File "/home/summa/chia-blockchain/chia/server/server.py", line 598, in wrapped_coroutine result = await coroutine File "/home/summa/chia-blockchain/chia/full_node/full_node_api.py", line 114, in new_peak waiter_count = len(self.full_node.new_peak_sem._waiters) TypeError: object of type 'NoneType' has no len() 2022-10-12T20:03:59.371 full_node full_node_server : ERROR Exception: object of type 'NoneType' has no len() <class 'TypeError'>, closing connection {'host': '65.34.144.6', 'port': 8444}. Traceback (most recent call last): File "/home/summa/chia-blockchain/chia/server/server.py", line 608, in api_call response: Optional[Message] = await asyncio.wait_for(wrapped_coroutine(), timeout=timeout) File "/usr/lib/python3.10/asyncio/tasks.py", line 408, in wait_for return await fut File "/home/summa/chia-blockchain/chia/server/server.py", line 605, in wrapped_coroutine raise e File "/home/summa/chia-blockchain/chia/server/server.py", line 598, in wrapped_coroutine result = await coroutine File "/home/summa/chia-blockchain/chia/full_node/full_node_api.py", line 114, in new_peak waiter_count = len(self.full_node.new_peak_sem._waiters) TypeError: object of type 'NoneType' has no len() 2022-10-12T20:03:59.487 full_node full_node_server : INFO Connection closed: 65.34.144.6, node id: 506fe4c05ce6b72bb707471842e552307c7a547aa9ba981175db5c08fa3e47e6 ``` * add LimitedSemaphore
1 parent bc6371a commit 8ff73e9

File tree

4 files changed

+113
-26
lines changed

4 files changed

+113
-26
lines changed

chia/full_node/full_node.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
from chia.util.db_wrapper import DBWrapper2, manage_connection
7373
from chia.util.errors import ConsensusError, Err, ValidationError
7474
from chia.util.ints import uint8, uint32, uint64, uint128
75+
from chia.util.limited_semaphore import LimitedSemaphore
7576
from chia.util.path import path_from_root
7677
from chia.util.safe_cancel_task import cancel_task_safe
7778
from chia.util.profiler import profile_task
@@ -124,8 +125,8 @@ class FullNode:
124125
simulator_transaction_callback: Optional[Callable[[bytes32], Awaitable[None]]]
125126
_sync_task: Optional[asyncio.Task[None]]
126127
_transaction_queue: Optional[asyncio.PriorityQueue[Tuple[int, TransactionQueueEntry]]]
127-
_compact_vdf_sem: Optional[asyncio.Semaphore]
128-
_new_peak_sem: Optional[asyncio.Semaphore]
128+
_compact_vdf_sem: Optional[LimitedSemaphore]
129+
_new_peak_sem: Optional[LimitedSemaphore]
129130
_respond_transaction_semaphore: Optional[asyncio.Semaphore]
130131
_db_wrapper: Optional[DBWrapper2]
131132
_hint_store: Optional[HintStore]
@@ -266,12 +267,12 @@ def hint_store(self) -> HintStore:
266267
return self._hint_store
267268

268269
@property
269-
def new_peak_sem(self) -> asyncio.Semaphore:
270+
def new_peak_sem(self) -> LimitedSemaphore:
270271
assert self._new_peak_sem is not None
271272
return self._new_peak_sem
272273

273274
@property
274-
def compact_vdf_sem(self) -> asyncio.Semaphore:
275+
def compact_vdf_sem(self) -> LimitedSemaphore:
275276
assert self._compact_vdf_sem is not None
276277
return self._compact_vdf_sem
277278

@@ -313,11 +314,11 @@ def _set_state_changed_callback(self, callback: Callable[..., Any]) -> None:
313314

314315
async def _start(self) -> None:
315316
self._timelord_lock = asyncio.Lock()
316-
self._compact_vdf_sem = asyncio.Semaphore(4)
317+
self._compact_vdf_sem = LimitedSemaphore.create(active_limit=4, waiting_limit=20)
317318

318319
# We don't want to run too many concurrent new_peak instances, because it would fetch the same block from
319320
# multiple peers and re-validate.
320-
self._new_peak_sem = asyncio.Semaphore(2)
321+
self._new_peak_sem = LimitedSemaphore.create(active_limit=2, waiting_limit=20)
321322

322323
# These many respond_transaction tasks can be active at any point in time
323324
self._respond_transaction_semaphore = asyncio.Semaphore(200)

chia/full_node/full_node_api.py

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
from chia.util.generator_tools import get_block_header, tx_removals_and_additions
5656
from chia.util.hash import std_hash
5757
from chia.util.ints import uint8, uint32, uint64, uint128
58+
from chia.util.limited_semaphore import LimitedSemaphoreFullError
5859
from chia.util.merkle_set import MerkleSet
5960
from chia.full_node.mempool_check_conditions import get_name_puzzle_conditions
6061

@@ -120,17 +121,12 @@ async def new_peak(self, request: full_node_protocol.NewPeak, peer: ws.WSChiaCon
120121
"""
121122
# this semaphore limits the number of tasks that can call new_peak() at
122123
# the same time, since it can be expensive
123-
new_peak_sem = self.full_node.new_peak_sem
124-
waiter_count = 0 if new_peak_sem._waiters is None else len(new_peak_sem._waiters)
125-
126-
if waiter_count > 0:
127-
self.full_node.log.debug(f"new_peak Waiters: {waiter_count}")
128-
129-
if waiter_count > 20:
124+
try:
125+
async with self.full_node.new_peak_sem.acquire():
126+
await self.full_node.new_peak(request, peer)
127+
except LimitedSemaphoreFullError:
130128
return None
131129

132-
async with new_peak_sem:
133-
await self.full_node.new_peak(request, peer)
134130
return None
135131

136132
@api_request(peer_required=True)
@@ -1411,12 +1407,6 @@ async def new_compact_vdf(
14111407
if self.full_node.sync_store.get_sync_mode():
14121408
return None
14131409

1414-
compact_vdf_sem = self.full_node.compact_vdf_sem
1415-
waiter_count = 0 if compact_vdf_sem._waiters is None else len(compact_vdf_sem._waiters)
1416-
if waiter_count > 20:
1417-
self.log.debug(f"Ignoring NewCompactVDF: {request}, _waiters")
1418-
return None
1419-
14201410
name = std_hash(request_bytes)
14211411
if name in self.full_node.compact_vdf_requests:
14221412
self.log.debug(f"Ignoring NewCompactVDF: {request}, already requested")
@@ -1425,11 +1415,16 @@ async def new_compact_vdf(
14251415

14261416
# this semaphore will only allow a limited number of tasks call
14271417
# new_compact_vdf() at a time, since it can be expensive
1428-
async with self.full_node.compact_vdf_sem:
1429-
try:
1430-
await self.full_node.new_compact_vdf(request, peer)
1431-
finally:
1432-
self.full_node.compact_vdf_requests.remove(name)
1418+
try:
1419+
async with self.full_node.compact_vdf_sem.acquire():
1420+
try:
1421+
await self.full_node.new_compact_vdf(request, peer)
1422+
finally:
1423+
self.full_node.compact_vdf_requests.remove(name)
1424+
except LimitedSemaphoreFullError:
1425+
self.log.debug(f"Ignoring NewCompactVDF: {request}, _waiters")
1426+
return None
1427+
14331428
return None
14341429

14351430
@api_request(peer_required=True, reply_types=[ProtocolMessageTypes.respond_compact_vdf])

chia/util/limited_semaphore.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
from __future__ import annotations
2+
3+
import asyncio
4+
import contextlib
5+
from dataclasses import dataclass
6+
from typing import AsyncIterator
7+
8+
from typing_extensions import final
9+
10+
11+
class LimitedSemaphoreFullError(Exception):
12+
def __init__(self) -> None:
13+
super().__init__("no waiting slot available")
14+
15+
16+
@final
17+
@dataclass
18+
class LimitedSemaphore:
19+
_semaphore: asyncio.Semaphore
20+
_available_count: int
21+
22+
@classmethod
23+
def create(cls, active_limit: int, waiting_limit: int) -> LimitedSemaphore:
24+
return cls(
25+
_semaphore=asyncio.Semaphore(active_limit),
26+
_available_count=active_limit + waiting_limit,
27+
)
28+
29+
@contextlib.asynccontextmanager
30+
async def acquire(self) -> AsyncIterator[int]:
31+
if self._available_count < 1:
32+
raise LimitedSemaphoreFullError()
33+
34+
self._available_count -= 1
35+
try:
36+
async with self._semaphore:
37+
yield self._available_count
38+
finally:
39+
self._available_count += 1

tests/util/test_limited_semaphore.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
from __future__ import annotations
2+
3+
import asyncio
4+
from typing import Optional
5+
6+
import pytest
7+
8+
from chia.util.limited_semaphore import LimitedSemaphore, LimitedSemaphoreFullError
9+
10+
11+
@pytest.mark.asyncio
12+
async def test_stuff() -> None:
13+
active_limit = 2
14+
waiting_limit = 4
15+
total_limit = active_limit + waiting_limit
16+
beyond_limit = 3
17+
semaphore = LimitedSemaphore.create(active_limit=active_limit, waiting_limit=waiting_limit)
18+
finish_event = asyncio.Event()
19+
20+
async def acquire(entered_event: Optional[asyncio.Event] = None) -> None:
21+
async with semaphore.acquire():
22+
assert entered_event is not None
23+
entered_event.set()
24+
await finish_event.wait()
25+
26+
entered_events = [asyncio.Event() for _ in range(active_limit)]
27+
waiting_events = [asyncio.Event() for _ in range(waiting_limit)]
28+
failed_events = [asyncio.Event() for _ in range(beyond_limit)]
29+
30+
entered_tasks = [asyncio.create_task(acquire(entered_event=event)) for event in entered_events]
31+
waiting_tasks = [asyncio.create_task(acquire(entered_event=event)) for event in waiting_events]
32+
33+
await asyncio.gather(*(event.wait() for event in entered_events))
34+
assert all(event.is_set() for event in entered_events)
35+
assert all(not event.is_set() for event in waiting_events)
36+
37+
assert semaphore._available_count == 0
38+
39+
failure_tasks = [asyncio.create_task(acquire()) for _ in range(beyond_limit)]
40+
41+
failure_results = await asyncio.gather(*failure_tasks, return_exceptions=True)
42+
assert [str(error) for error in failure_results] == [str(LimitedSemaphoreFullError())] * beyond_limit
43+
assert all(not event.is_set() for event in failed_events)
44+
45+
assert semaphore._available_count == 0
46+
47+
finish_event.set()
48+
success_results = await asyncio.gather(*entered_tasks, *waiting_tasks)
49+
assert all(event.is_set() for event in waiting_events)
50+
assert success_results == [None] * total_limit
51+
52+
assert semaphore._available_count == total_limit

0 commit comments

Comments
 (0)