fix the campaigns concurrency test by fixing what it was catching
ExecuteCampaignRealConcurrencyTests had been written off as a known flake for weeks — about 45% red on a full-suite run. It was not timing noise. It was reporting a real defect, and the defect is in production code. execute_campaign looped with .iterator(), which keeps the read cursor on the user table open for the whole loop — a loop that writes a CampaignDelivery per user and, on the sms branch, makes a network call to Kavenegar in between. On SQLite that open cursor locks the table against any concurrent writer, and the lock is not released until the loop ends. So _retry_on_sqlite_lock could never recover from it no matter how long it waited: widening its window from 0.5s to 2s (also done here, since the linear backoff was too tight regardless) moved the failure rate not at all on its own. Both branches now materialise the ids before the first write. After the change: 20 consecutive runs of the test itself and 3 full-suite runs, all green. This is not only a test fix. While we are on SQLite the same pattern produces the same lock in production, where it surfaces as "database is locked" on unrelated requests that happen to land during a campaign run. Also in this pass ----------------- - CLAUDE.md carried three sections each claiming to be the current state, with three different question counts (1,441 / 1,783 / reality 2,463). There is now one current-state section and the older two are labelled as history, so whoever picks this up next is not reading stale numbers as fact. Three other figures that had drifted are corrected against the database: the verdict split, the figure-gap share (881/~42% is now 508/~14% after the five books were re-read), and the node coverage. - Dropped four genuinely unused imports. The remaining pyflakes hits are the signal registrations in apps.py, which are imported for effect. - .env.example suggested gpt-4o for vision; gpt-4.1-mini measured both cheaper and more accurate and is what the server actually runs. Audited all 2,463 questions across every grade: no question with the wrong number of options, none with zero or two correct answers, no empty or duplicated option, no orphan lesson or node, no duplicate question inside a lesson. 458 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -20,16 +20,20 @@ from notifications.services import notify
|
||||
from .models import CampaignDelivery, MessageCampaign
|
||||
|
||||
|
||||
def _retry_on_sqlite_lock(fn, *, retries=5, base_delay=0.05):
|
||||
def _retry_on_sqlite_lock(fn, *, retries=8, base_delay=0.05, max_delay=0.4):
|
||||
"""
|
||||
روی SQLite (مخصوصاً دیتابیسِ حافظهایِ shared-cache که تستها ازش
|
||||
استفاده میکنن)، وقتی دو اتصالِ همزمان میخوان همون جدول رو بنویسن،
|
||||
ممکنه OperationalError('database table is locked') بگیریم. این
|
||||
SQLITE_LOCKEDه نه SQLITE_BUSY، پس busy_timeoutِ معمولیِ اتصال کمکی
|
||||
نمیکنه و بلافاصله raise میشه؛ یک خطای واقعیِ داده نیست، فقط
|
||||
تناقضِ لحظهایِ لاکه. این تابع فقط همین خطای گذرا رو با چند بار
|
||||
تلاشِ سریع (backoffِ کوچیکِ فزاینده) دوباره امتحان میکنه؛ هر
|
||||
روی SQLite وقتی دو اتصالِ همزمان میخوان همون جدول رو بنویسن، ممکنه
|
||||
OperationalError('database table is locked') بگیریم. این SQLITE_LOCKEDه
|
||||
نه SQLITE_BUSY، پس busy_timeoutِ اتصال کمکی نمیکنه و بلافاصله raise
|
||||
میشه؛ خطای واقعیِ داده نیست، فقط تناقضِ لحظهایِ لاک. هر
|
||||
OperationalError دیگه (یا شکستِ تلاشِ آخر) دستنخورده بالا میره.
|
||||
|
||||
⚠️ پنجرهی انتظار باید از خودِ قفل درازتر باشد، وگرنه محافظ فقط *گاهی*
|
||||
کار میکند. نسخهی اول ۵ تلاش با backoffِ خطی داشت — رویِهم ۰٫۵ ثانیه —
|
||||
و تستِ همزمانیِ campaigns را در حدودِ نیمی از اجراهای کاملِ سوئیت قرمز
|
||||
میکرد، چون زیرِ بارِ کلِ تستها قفل بیشتر از آن طول میکشد. حالا
|
||||
backoff نمایی است و تا ~۲ ثانیه صبر میکند؛ سقفِ max_delay هم هست تا
|
||||
یک قفلِ طولانی، درخواستِ کاربر را بینهایت معلق نگه ندارد.
|
||||
"""
|
||||
for attempt in range(retries):
|
||||
try:
|
||||
@@ -37,7 +41,7 @@ def _retry_on_sqlite_lock(fn, *, retries=5, base_delay=0.05):
|
||||
except OperationalError as exc:
|
||||
if 'locked' not in str(exc).lower() or attempt == retries - 1:
|
||||
raise
|
||||
time.sleep(base_delay * (attempt + 1))
|
||||
time.sleep(min(base_delay * (2 ** attempt), max_delay))
|
||||
|
||||
# عمداً به notifications.models.Notification.TYPE_CHOICES اضافه نشده: آن
|
||||
# choices فقط رویدادهاییه که notifications بهعنوانِ listener گوش میده،
|
||||
@@ -220,16 +224,26 @@ def execute_campaign(campaign: MessageCampaign) -> None:
|
||||
already_sent_user_ids = _already_sent_user_ids(campaign)
|
||||
|
||||
if campaign.channel == 'in_app':
|
||||
for user in users.only('id').iterator():
|
||||
if user.id in already_sent_user_ids:
|
||||
# ⚠️ عمداً list() و نه .iterator(): کرسرِ باز باید *پیش از* اولین
|
||||
# نوشتن بسته شود. با .iterator() خواندنِ کاربرها در تمامِ طولِ حلقه
|
||||
# باز میماند و روی SQLite همان، جدول را برای هر نویسندهی همزمان
|
||||
# قفل میکند — قفلی که تا پایانِ حلقه رها نمیشود، پس هیچ مقدار
|
||||
# retry هم نجاتش نمیدهد. این دقیقاً همان چیزی بود که تستِ
|
||||
# همزمانیِ campaigns را در حدودِ نیمی از اجراها قرمز میکرد.
|
||||
#
|
||||
# حافظه: اینجا فقط idها نگه داشته میشوند و این ماژول طبقِ
|
||||
# docstringِ بالا برای لیستهای کوچک است، نه کمپینِ انبوه.
|
||||
for user_id in list(users.values_list('id', flat=True)):
|
||||
if user_id in already_sent_user_ids:
|
||||
continue
|
||||
delivery = _claim_delivery(campaign, user.id)
|
||||
delivery = _claim_delivery(campaign, user_id)
|
||||
if delivery is None:
|
||||
continue
|
||||
def _notify_and_mark_sent():
|
||||
|
||||
def _notify_and_mark_sent(user_id=user_id, delivery=delivery):
|
||||
with transaction.atomic():
|
||||
notify(
|
||||
user_id=user.id,
|
||||
user_id=user_id,
|
||||
type=CAMPAIGN_NOTIFICATION_TYPE,
|
||||
title=campaign.title,
|
||||
body=campaign.body,
|
||||
@@ -251,16 +265,19 @@ def execute_campaign(campaign: MessageCampaign) -> None:
|
||||
return
|
||||
|
||||
if campaign.channel == 'sms':
|
||||
for user in users.only('id', 'phone').iterator():
|
||||
if user.id in already_sent_user_ids:
|
||||
# همان دلیلِ شاخهی in_app بالا، و اینجا حتی مهمتر: وسطِ این حلقه
|
||||
# یک فراخوانیِ شبکهای به کاوهنگار انجام میشود، پس کرسرِ باز
|
||||
# میتوانست بهاندازهی کلِ ارسالِ پیامکها جدول را قفل نگه دارد.
|
||||
for user_id, phone in list(users.values_list('id', 'phone')):
|
||||
if user_id in already_sent_user_ids:
|
||||
continue
|
||||
if not user.phone:
|
||||
if not phone:
|
||||
continue
|
||||
delivery = _claim_delivery(campaign, user.id)
|
||||
delivery = _claim_delivery(campaign, user_id)
|
||||
if delivery is None:
|
||||
continue
|
||||
try:
|
||||
account_services.send_sms(phone=user.phone, message=campaign.body)
|
||||
account_services.send_sms(phone=phone, message=campaign.body)
|
||||
except SMSDeliveryError as exc:
|
||||
_mark_delivery_result(delivery, error=exc)
|
||||
else:
|
||||
|
||||
Reference in New Issue
Block a user