Skip to content

Commit 2a75aad

Browse files
committed
Optimize the increment canvas count so that it is non blocking
1 parent 94e0982 commit 2a75aad

5 files changed

Lines changed: 303 additions & 13 deletions

File tree

backend/routes/export.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,9 @@ def import_canvas(room_id):
408408
if field in stroke:
409409
stroke_data[field] = stroke[field]
410410

411-
# Generate unique stroke ID
412-
draw_count = get_canvas_draw_count()
411+
# ATOMIC OPERATION: Increment counter and get the NEW value
412+
# This must happen FIRST to ensure unique stroke IDs even under concurrent load
413+
draw_count = increment_canvas_draw_count()
413414
stroke_id = f"res-canvas-draw-{draw_count}"
414415
stroke_data["id"] = stroke_id
415416

@@ -486,7 +487,7 @@ def import_canvas(room_id):
486487
logger.warning(f"import_canvas: GraphQL commit failed for {stroke_id}: {e}")
487488
# Continue even if GraphQL fails - data is in Redis/MongoDB
488489

489-
increment_canvas_draw_count()
490+
# Note: increment_canvas_draw_count() already called above - do not call again
490491
imported_count += 1
491492

492493
except Exception as e:

backend/routes/new_line.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ def submit_new_line():
3535
if original_ids:
3636
redis_client.sadd("cut-stroke-ids", *original_ids)
3737

38-
res_canvas_draw_count = get_canvas_draw_count()
38+
# ATOMIC OPERATION: Increment counter and get the NEW value
39+
# This must happen FIRST to ensure unique stroke IDs even under concurrent load
40+
res_canvas_draw_count = increment_canvas_draw_count()
3941
request_data['id'] = "res-canvas-draw-" + str(res_canvas_draw_count)
4042
request_data.pop('undone', None)
4143

@@ -62,7 +64,7 @@ def submit_new_line():
6264
txn_id = commit_transaction_via_graphql(prep)
6365
request_data['txnId'] = txn_id
6466

65-
increment_canvas_draw_count()
67+
# Note: increment_canvas_draw_count() already called above - do not call again
6668
cache_entry = full_data.copy()
6769
cache_entry['txnId'] = txn_id
6870
redis_client.set(cache_entry['id'], json.dumps(cache_entry))

backend/routes/submit_room_line.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ def submit_room_line():
119119
logger.exception('signature verification failed')
120120
return jsonify({'status': 'error', 'message': 'bad signature'}), 400
121121

122-
draw_count = get_canvas_draw_count()
122+
# Increment counter and get the NEW value
123+
# This must happen FIRST to ensure unique stroke IDs even under concurrent load
124+
# The counter is now incremented atomically (redis.incr) with no lock
125+
draw_count = increment_canvas_draw_count()
123126
stroke_id = f"res-canvas-draw-{draw_count}"
124127
drawing['id'] = drawing.get('id') or stroke_id
125128
drawing.pop('undone', None)
@@ -134,7 +137,6 @@ def submit_room_line():
134137
"roomId": roomId,
135138
}
136139
redis_client.set(stroke_id, json.dumps(cache_entry))
137-
increment_canvas_draw_count()
138140

139141
try:
140142
parsed = data.get('value')

backend/services/canvas_counter.py

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
# services/canvas_counter.py
22

3-
from services.db import redis_client, strokes_coll, lock
3+
from services.db import redis_client, strokes_coll
44
from services.graphql_service import commit_transaction_via_graphql
55
from config import *
66
import logging
7+
import threading
78

89
logger = logging.getLogger(__name__)
910

1011
def get_canvas_draw_count():
12+
"""
13+
Get the current canvas draw count.
14+
Returns the count from Redis (fast path) or MongoDB (fallback).
15+
"""
1116
count = redis_client.get('res-canvas-draw-count')
1217

1318
if count is None:
@@ -35,11 +40,39 @@ def get_canvas_draw_count():
3540
return count
3641

3742
def increment_canvas_draw_count():
38-
with lock:
39-
count = get_canvas_draw_count() + 1
40-
41-
redis_client.set('res-canvas-draw-count', count)
43+
"""
44+
Atomically increment the canvas draw count and return the NEW value.
45+
46+
Uses redis.incr() which is atomic and thread-safe.
47+
The GraphQL commit happens asynchronously to avoid blocking stroke submissions.
4248
49+
Returns:
50+
int: The NEW counter value (already incremented)
51+
"""
52+
count = redis_client.incr('res-canvas-draw-count')
53+
54+
try:
55+
threading.Thread(
56+
target=_async_commit_counter_to_blockchain,
57+
args=(count,),
58+
daemon=True
59+
).start()
60+
except Exception as e:
61+
logger.error(f"Failed to start async counter commit thread: {e}")
62+
63+
return count
64+
65+
def _async_commit_counter_to_blockchain(count: int):
66+
"""
67+
Background thread that commits the counter value to ResilientDB via GraphQL.
68+
69+
This happens asynchronously so stroke submissions aren't blocked by blockchain latency.
70+
If GraphQL is down, the commit is queued for retry.
71+
72+
Args:
73+
count: The counter value to commit
74+
"""
75+
try:
4376
increment_count = {
4477
"operation": "CREATE",
4578
"amount": 1,
@@ -54,5 +87,21 @@ def increment_canvas_draw_count():
5487
}
5588
}
5689
commit_transaction_via_graphql(increment_count)
90+
logger.debug(f"Counter {count} committed to blockchain successfully")
91+
92+
except Exception as e:
93+
logger.warning(f"Failed to commit counter {count} to blockchain: {e}")
94+
try:
95+
from services.graphql_retry_queue import add_to_retry_queue
96+
add_to_retry_queue(
97+
f"counter-{count}",
98+
{
99+
"id": "res-canvas-draw-count",
100+
"value": count,
101+
"type": "counter_increment"
102+
}
103+
)
104+
logger.info(f"Counter {count} queued for retry")
105+
except Exception as retry_error:
106+
logger.error(f"Failed to queue counter {count} for retry: {retry_error}")
57107

58-
return count
Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
"""
2+
Test concurrent stroke submissions to verify atomic counter prevents race conditions.
3+
4+
This test simulates multiple users submitting strokes simultaneously to ensure:
5+
1. No duplicate stroke IDs are generated
6+
2. Counter increments correctly under concurrent load
7+
3. Race conditions are eliminated by atomic redis.incr()
8+
4. System can handle 10+ concurrent users (vs 2-3 before optimization)
9+
"""
10+
11+
import pytest
12+
import sys
13+
import os
14+
import threading
15+
import time
16+
from concurrent.futures import ThreadPoolExecutor, as_completed
17+
18+
# Add backend to path
19+
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
20+
21+
from services.canvas_counter import increment_canvas_draw_count, get_canvas_draw_count
22+
from services.db import redis_client
23+
24+
25+
@pytest.fixture(autouse=True)
26+
def reset_counter():
27+
"""Reset counter before each test."""
28+
redis_client.set('res-canvas-draw-count', 0)
29+
yield
30+
# Cleanup after test
31+
redis_client.delete('res-canvas-draw-count')
32+
33+
34+
def submit_stroke_simulation(user_id, delay=0):
35+
"""
36+
Simulate a single user submitting a stroke.
37+
38+
Args:
39+
user_id: Identifier for the simulated user
40+
delay: Optional delay before submission (simulates network latency)
41+
42+
Returns:
43+
tuple: (user_id, stroke_id, draw_count)
44+
"""
45+
if delay > 0:
46+
time.sleep(delay)
47+
48+
# This is the critical path - increment counter and get stroke ID
49+
draw_count = increment_canvas_draw_count()
50+
stroke_id = f"res-canvas-draw-{draw_count}"
51+
52+
return (user_id, stroke_id, draw_count)
53+
54+
55+
def test_sequential_counter_increments():
56+
"""Test that sequential submissions work correctly (baseline)."""
57+
results = []
58+
for i in range(10):
59+
_, stroke_id, count = submit_stroke_simulation(f"user_{i}")
60+
results.append((stroke_id, count))
61+
62+
# Verify all IDs are unique and sequential
63+
stroke_ids = [r[0] for r in results]
64+
counts = [r[1] for r in results]
65+
66+
assert len(stroke_ids) == len(set(stroke_ids)), "Duplicate stroke IDs in sequential test"
67+
assert counts == list(range(1, 11)), f"Counts not sequential: {counts}"
68+
69+
# Verify final counter value
70+
final_count = get_canvas_draw_count()
71+
assert final_count == 10, f"Final count should be 10, got {final_count}"
72+
73+
74+
def test_concurrent_10_users():
75+
"""Test 10 users submitting strokes simultaneously."""
76+
num_users = 10
77+
results = []
78+
79+
with ThreadPoolExecutor(max_workers=num_users) as executor:
80+
futures = [
81+
executor.submit(submit_stroke_simulation, f"user_{i}")
82+
for i in range(num_users)
83+
]
84+
85+
for future in as_completed(futures):
86+
results.append(future.result())
87+
88+
# Extract stroke IDs and counts
89+
stroke_ids = [r[1] for r in results]
90+
counts = [r[2] for r in results]
91+
92+
# Critical assertions
93+
assert len(stroke_ids) == num_users, f"Expected {num_users} strokes, got {len(stroke_ids)}"
94+
assert len(set(stroke_ids)) == num_users, f"RACE CONDITION: Duplicate stroke IDs found! {stroke_ids}"
95+
assert len(set(counts)) == num_users, f"RACE CONDITION: Duplicate counts found! {counts}"
96+
assert set(counts) == set(range(1, num_users + 1)), f"Counts not in expected range: {counts}"
97+
98+
# Verify final counter
99+
final_count = get_canvas_draw_count()
100+
assert final_count == num_users, f"Final count should be {num_users}, got {final_count}"
101+
102+
103+
def test_concurrent_50_users():
104+
"""Test 50 users submitting strokes simultaneously (target capacity)."""
105+
num_users = 50
106+
results = []
107+
108+
with ThreadPoolExecutor(max_workers=num_users) as executor:
109+
futures = [
110+
executor.submit(submit_stroke_simulation, f"user_{i}")
111+
for i in range(num_users)
112+
]
113+
114+
for future in as_completed(futures):
115+
results.append(future.result())
116+
117+
# Extract stroke IDs and counts
118+
stroke_ids = [r[1] for r in results]
119+
counts = [r[2] for r in results]
120+
121+
# Critical assertions
122+
assert len(stroke_ids) == num_users, f"Expected {num_users} strokes, got {len(stroke_ids)}"
123+
assert len(set(stroke_ids)) == num_users, f"RACE CONDITION: Duplicate stroke IDs at scale! {len(stroke_ids) - len(set(stroke_ids))} duplicates"
124+
assert len(set(counts)) == num_users, f"RACE CONDITION: Duplicate counts at scale! {len(counts) - len(set(counts))} duplicates"
125+
assert set(counts) == set(range(1, num_users + 1)), f"Counts not in expected range 1-{num_users}"
126+
127+
# Verify final counter
128+
final_count = get_canvas_draw_count()
129+
assert final_count == num_users, f"Final count should be {num_users}, got {final_count}"
130+
131+
132+
def test_concurrent_100_users():
133+
"""Test 100 users submitting strokes simultaneously (stretch goal)."""
134+
num_users = 100
135+
results = []
136+
137+
start_time = time.time()
138+
139+
with ThreadPoolExecutor(max_workers=num_users) as executor:
140+
futures = [
141+
executor.submit(submit_stroke_simulation, f"user_{i}")
142+
for i in range(num_users)
143+
]
144+
145+
for future in as_completed(futures):
146+
results.append(future.result())
147+
148+
end_time = time.time()
149+
elapsed = end_time - start_time
150+
151+
# Extract stroke IDs and counts
152+
stroke_ids = [r[1] for r in results]
153+
counts = [r[2] for r in results]
154+
155+
# Critical assertions
156+
assert len(stroke_ids) == num_users, f"Expected {num_users} strokes, got {len(stroke_ids)}"
157+
assert len(set(stroke_ids)) == num_users, f"RACE CONDITION: Duplicate stroke IDs at 100 users! {len(stroke_ids) - len(set(stroke_ids))} duplicates"
158+
assert len(set(counts)) == num_users, f"RACE CONDITION: Duplicate counts at 100 users!"
159+
160+
# Performance assertion: should complete in reasonable time with atomic operations
161+
# With old lock: ~8 seconds (80ms * 100), with atomic: < 1 second
162+
print(f"\n100 concurrent users completed in {elapsed:.2f} seconds")
163+
assert elapsed < 5.0, f"Too slow: {elapsed:.2f}s (atomic operations should be fast)"
164+
165+
# Verify final counter
166+
final_count = get_canvas_draw_count()
167+
assert final_count == num_users, f"Final count should be {num_users}, got {final_count}"
168+
169+
170+
def test_concurrent_with_network_latency():
171+
"""Test concurrent submissions with simulated network latency."""
172+
num_users = 20
173+
results = []
174+
175+
with ThreadPoolExecutor(max_workers=num_users) as executor:
176+
futures = [
177+
executor.submit(submit_stroke_simulation, f"user_{i}", delay=0.01 * (i % 5))
178+
for i in range(num_users)
179+
]
180+
181+
for future in as_completed(futures):
182+
results.append(future.result())
183+
184+
# Extract stroke IDs and counts
185+
stroke_ids = [r[1] for r in results]
186+
counts = [r[2] for r in results]
187+
188+
# Critical assertions
189+
assert len(set(stroke_ids)) == num_users, f"RACE CONDITION: Duplicate stroke IDs with latency!"
190+
assert len(set(counts)) == num_users, f"RACE CONDITION: Duplicate counts with latency!"
191+
assert set(counts) == set(range(1, num_users + 1)), "Counts not in expected range"
192+
193+
194+
def test_counter_atomicity_stress():
195+
"""
196+
Stress test: Verify atomicity under extreme concurrent load.
197+
198+
This test hammers the counter with many rapid increments from multiple threads
199+
to detect any race conditions in the atomic redis.incr() implementation.
200+
"""
201+
num_increments = 1000
202+
num_threads = 10
203+
results = []
204+
205+
def rapid_fire_increments(thread_id, count):
206+
"""Perform multiple rapid increments from a single thread."""
207+
thread_results = []
208+
for i in range(count):
209+
draw_count = increment_canvas_draw_count()
210+
thread_results.append(draw_count)
211+
return thread_results
212+
213+
increments_per_thread = num_increments // num_threads
214+
215+
with ThreadPoolExecutor(max_workers=num_threads) as executor:
216+
futures = [
217+
executor.submit(rapid_fire_increments, tid, increments_per_thread)
218+
for tid in range(num_threads)
219+
]
220+
221+
for future in as_completed(futures):
222+
results.extend(future.result())
223+
224+
# Critical assertions
225+
assert len(results) == num_increments, f"Expected {num_increments} increments, got {len(results)}"
226+
assert len(set(results)) == num_increments, f"RACE CONDITION: {len(results) - len(set(results))} duplicate counts!"
227+
assert set(results) == set(range(1, num_increments + 1)), "Some counts missing or duplicated"
228+
229+
# Verify final counter
230+
final_count = get_canvas_draw_count()
231+
assert final_count == num_increments, f"Final count should be {num_increments}, got {final_count}"
232+
233+
234+
if __name__ == "__main__":
235+
# Run tests with verbose output
236+
pytest.main([__file__, "-v", "-s"])

0 commit comments

Comments
 (0)