mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-20 17:50:12 +02:00
Start queued jobs in list order, whether pinned or "Any <model>" (#3200)
This commit is contained in:
@@ -54,6 +54,7 @@ from backend.app.services.print_batch import (
|
||||
refresh_batch_status_for_item,
|
||||
)
|
||||
from backend.app.services.print_cost_estimate import estimate_queue_source_cost
|
||||
from backend.app.services.queue_position import lock_queue_positions, max_queue_position
|
||||
from backend.app.utils.printer_models import (
|
||||
is_gcode_compatible,
|
||||
)
|
||||
@@ -1001,47 +1002,14 @@ async def add_to_queue(
|
||||
await db.flush() # Get batch.id before creating items
|
||||
batch_id = batch.id
|
||||
|
||||
# Get queue scope for this printer (or for unassigned/model-based items).
|
||||
if data.printer_id is not None:
|
||||
queue_scope = (
|
||||
PrintQueueItem.printer_id == data.printer_id,
|
||||
PrintQueueItem.status == "pending",
|
||||
)
|
||||
else:
|
||||
# For unassigned/model-based items, scope across all unassigned.
|
||||
queue_scope = (
|
||||
PrintQueueItem.printer_id.is_(None),
|
||||
PrintQueueItem.status == "pending",
|
||||
)
|
||||
|
||||
# Serialize concurrent queue inserts to the same scope (#1625-followup).
|
||||
# The race: two concurrent ASAP inserts both compute MAX(position) before
|
||||
# either commits; in an empty scope, both INSERT at position 1 (duplicate).
|
||||
# In a non-empty scope, Postgres's row-level locks on the UPDATE shift
|
||||
# serialize naturally, but the empty-scope path has no rows to lock.
|
||||
# A transaction-scoped advisory lock keyed on the printer_id closes that
|
||||
# window; the lock is released automatically at commit/rollback. Different
|
||||
# printers don't contend. SQLite serializes writes implicitly so this is a
|
||||
# no-op there.
|
||||
#
|
||||
# Dialect is checked against the actual session binding, NOT the
|
||||
# `is_sqlite()` helper, because the test fixture overrides `get_db` with a
|
||||
# SQLite engine while `settings.database_url` still points at Postgres
|
||||
# (the helper reads settings). Inspecting the connection directly is the
|
||||
# right shape for any code that mutates SQL based on the live dialect.
|
||||
from sqlalchemy import text
|
||||
|
||||
bind = db.get_bind()
|
||||
if bind.dialect.name == "postgresql":
|
||||
scope_key = data.printer_id if data.printer_id is not None else 0
|
||||
# 1625 namespaces the lock so it can't collide with other advisory
|
||||
# locks elsewhere in the codebase.
|
||||
await db.execute(text("SELECT pg_advisory_xact_lock(1625, :k)"), {"k": scope_key})
|
||||
# Positions are one sequence across every pending item (#3200), so a new
|
||||
# item lands relative to the whole list, not to its printer's share of it.
|
||||
queue_scope = (PrintQueueItem.status == "pending",)
|
||||
await lock_queue_positions(db)
|
||||
|
||||
insert_position = max(1, data.insert_position or 1)
|
||||
if data.insert_at_top or data.insert_position is not None:
|
||||
result = await db.execute(select(func.max(PrintQueueItem.position)).where(*queue_scope))
|
||||
max_pos = result.scalar() or 0
|
||||
max_pos = await max_queue_position(db)
|
||||
insert_position = min(insert_position, max_pos + 1)
|
||||
await db.execute(
|
||||
update(PrintQueueItem)
|
||||
@@ -1051,9 +1019,7 @@ async def add_to_queue(
|
||||
)
|
||||
start_position = insert_position
|
||||
else:
|
||||
result = await db.execute(select(func.max(PrintQueueItem.position)).where(*queue_scope))
|
||||
max_pos = result.scalar() or 0
|
||||
start_position = max_pos + 1
|
||||
start_position = await max_queue_position(db) + 1
|
||||
|
||||
# Resolve print_time_seconds for SJF scheduling (cache on item at creation)
|
||||
cached_print_time = None
|
||||
|
||||
@@ -14,6 +14,7 @@ from backend.app.models.printer import Printer
|
||||
from backend.app.schemas.printer import HMSErrorResponse, hms_error_responses
|
||||
from backend.app.services.print_confirmation import confirm_outcome_for_new_queue_item
|
||||
from backend.app.services.printer_manager import printer_manager
|
||||
from backend.app.services.queue_position import next_queue_position
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -114,18 +115,9 @@ async def webhook_add_to_queue(
|
||||
if not printer:
|
||||
raise HTTPException(status_code=404, detail="Printer not found")
|
||||
|
||||
# Get next position
|
||||
result = await db.execute(
|
||||
select(PrintQueueItem.position)
|
||||
.where(
|
||||
PrintQueueItem.printer_id == data.printer_id,
|
||||
PrintQueueItem.status == "pending",
|
||||
)
|
||||
.order_by(PrintQueueItem.position.desc())
|
||||
.limit(1)
|
||||
)
|
||||
max_position = result.scalar()
|
||||
next_position = (max_position or 0) + 1
|
||||
# Append to the end of the queue: positions are one sequence across all
|
||||
# pending items, not one per printer (#3200).
|
||||
next_position = await next_queue_position(db)
|
||||
|
||||
# Parse scheduled time if provided
|
||||
scheduled_time = None
|
||||
|
||||
@@ -80,7 +80,7 @@ class PrintQueueItemCreate(BaseModel):
|
||||
require_previous_success: bool = False
|
||||
auto_off_after: bool = False # Power off printer after print completes
|
||||
manual_start: bool = False # Requires manual trigger to start (staged)
|
||||
insert_at_top: bool = False # Insert ahead of other pending items in the same queue scope
|
||||
insert_at_top: bool = False # Insert ahead of other pending items (one queue order across all printers, #3200)
|
||||
insert_position: int | None = None # 1-indexed insertion position for priority queueing
|
||||
# Persistent "Print Anyway" acknowledgement (#1698-followup). When set,
|
||||
# PrintModal already showed the deficit warning and the user confirmed,
|
||||
|
||||
@@ -21,13 +21,14 @@ import logging
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import func, select, text
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy.orm import selectinload
|
||||
|
||||
from backend.app.models.print_batch import PrintBatch, PrintBatchPlate
|
||||
from backend.app.models.print_log import PrintLogEntry
|
||||
from backend.app.models.print_queue import PrintQueueItem, PrintQueueVariant
|
||||
from backend.app.services.queue_position import next_queue_position
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -432,34 +433,6 @@ async def refresh_batch_status_for_item(db: AsyncSession, queue_item_id: int) ->
|
||||
await refresh_batch_status(db, batch)
|
||||
|
||||
|
||||
async def _next_position(db: AsyncSession, printer_id: int | None) -> int:
|
||||
"""Next free queue position in the scope a clone will land in.
|
||||
|
||||
Positions are per-queue, not global: one sequence per printer plus one
|
||||
shared sequence for unassigned / model-based items, matching the scope the
|
||||
add-to-queue route uses. Taking a global MAX here would drop every clone
|
||||
at the end of whichever printer's queue happens to be longest and scramble
|
||||
the order the user sees.
|
||||
"""
|
||||
# Same advisory lock the add-to-queue route takes (#1625-followup): two
|
||||
# concurrent inserts into an empty scope would otherwise both read
|
||||
# MAX(position) as 0 and land on position 1. SQLite serialises writes
|
||||
# implicitly and needs no equivalent.
|
||||
bind = db.get_bind()
|
||||
if bind.dialect.name == "postgresql":
|
||||
await db.execute(
|
||||
text("SELECT pg_advisory_xact_lock(1625, :k)"), {"k": printer_id if printer_id is not None else 0}
|
||||
)
|
||||
|
||||
scope = PrintQueueItem.printer_id == printer_id if printer_id is not None else PrintQueueItem.printer_id.is_(None)
|
||||
max_pos = (
|
||||
await db.execute(
|
||||
select(func.max(PrintQueueItem.position)).where(scope).where(PrintQueueItem.status == "pending")
|
||||
)
|
||||
).scalar() or 0
|
||||
return max_pos + 1
|
||||
|
||||
|
||||
def _clone_queue_item(source: PrintQueueItem, *, position: int, created_by_id: int | None) -> PrintQueueItem:
|
||||
"""Copy *source*'s print configuration into a fresh pending item.
|
||||
|
||||
@@ -556,9 +529,9 @@ async def dispatch_remaining(
|
||||
if limit is not None:
|
||||
wanted = min(wanted, limit - len(created))
|
||||
|
||||
# One scope per source printer; clones for this plate all land in it,
|
||||
# appended after whatever is already queued there.
|
||||
position = await _next_position(db, source.printer_id)
|
||||
# Clones for this plate are appended to the end of the queue (#3200),
|
||||
# after whatever is already pending, in plate order.
|
||||
position = await next_queue_position(db)
|
||||
|
||||
for _ in range(wanted):
|
||||
clone = _clone_queue_item(source, position=position, created_by_id=created_by_id)
|
||||
|
||||
@@ -410,6 +410,27 @@ def _filament_constraints(candidate: _ModelCandidate) -> tuple[list[str] | None,
|
||||
return effective_types, filament_overrides
|
||||
|
||||
|
||||
def _could_take_printer(item: PrintQueueItem, printer_id: int, printer_model: str | None) -> bool:
|
||||
"""Whether ``item`` was competing for the printer another item just took.
|
||||
|
||||
The SJF starvation guard marks a job as jumped when a shorter one lower in
|
||||
the queue takes a printer it wanted. A pinned job and an "Any <model>" job
|
||||
compete for the same printer, so the guard has to look across both lanes
|
||||
(#3200); looking only inside the dispatched item's own lane let a stream of
|
||||
short model-based jobs hold a longer pinned job back indefinitely.
|
||||
|
||||
Location filters are not checked: over-marking only lifts an item that was
|
||||
going to wait anyway, while under-marking is the starvation this prevents.
|
||||
"""
|
||||
if item.printer_id is not None:
|
||||
return item.printer_id == printer_id
|
||||
if not printer_model:
|
||||
return False
|
||||
wanted = (normalize_printer_model(printer_model) or printer_model).lower()
|
||||
models = [v.target_model for v in item.variants] if item.variants else [item.target_model]
|
||||
return any(m and (normalize_printer_model(m) or m).lower() == wanted for m in models)
|
||||
|
||||
|
||||
def _candidates_for(item: PrintQueueItem) -> list[_ModelCandidate]:
|
||||
"""Candidate files for ``item``, best first.
|
||||
|
||||
@@ -1306,11 +1327,17 @@ class PrintScheduler:
|
||||
# Check if shortest-job-first scheduling is enabled
|
||||
sjf_enabled = await self._get_bool_setting(db, "queue_shortest_first")
|
||||
|
||||
# Get all pending items, ordered by printer and position (or SJF order)
|
||||
# Get all pending items in the one order the queue page shows them
|
||||
# in (#3200). The first eligible item takes a printer, so this order
|
||||
# decides who wins a printer that a pinned job and an "Any <model>"
|
||||
# job both want. It used to start with ``printer_id``, which made the
|
||||
# lane outrank the position: SQLite sorts NULL first, so model-based
|
||||
# jobs always won; PostgreSQL sorts it last, so pinned jobs did.
|
||||
# Neither is what the user dragged into place.
|
||||
if sjf_enabled:
|
||||
# SJF: group by printer (and target_model for model-based jobs),
|
||||
# then items already jumped get top priority (starvation guard),
|
||||
# then sort by print_time ascending. Items with no print time go last.
|
||||
# SJF: items already jumped get top priority (starvation guard),
|
||||
# then sort by print_time ascending. Items with no print time go
|
||||
# last, and position breaks ties.
|
||||
result = await db.execute(
|
||||
select(PrintQueueItem)
|
||||
.where(PrintQueueItem.status == "pending")
|
||||
@@ -1331,11 +1358,10 @@ class PrintScheduler:
|
||||
selectinload(PrintQueueItem.variants).selectinload(PrintQueueVariant.library_file),
|
||||
)
|
||||
.order_by(
|
||||
PrintQueueItem.printer_id,
|
||||
PrintQueueItem.target_model,
|
||||
PrintQueueItem.been_jumped.desc(),
|
||||
PrintQueueItem.print_time_seconds.asc().nullslast(),
|
||||
PrintQueueItem.position,
|
||||
PrintQueueItem.id,
|
||||
)
|
||||
)
|
||||
else:
|
||||
@@ -1352,7 +1378,7 @@ class PrintScheduler:
|
||||
# raise in async.
|
||||
selectinload(PrintQueueItem.variants).selectinload(PrintQueueVariant.library_file),
|
||||
)
|
||||
.order_by(PrintQueueItem.printer_id, PrintQueueItem.position)
|
||||
.order_by(PrintQueueItem.position, PrintQueueItem.id)
|
||||
)
|
||||
items = list(result.scalars().all())
|
||||
|
||||
@@ -1889,11 +1915,13 @@ class PrintScheduler:
|
||||
|
||||
# SJF starvation guard: mark items that were jumped
|
||||
if sjf_enabled and item.print_time_seconds is not None:
|
||||
pinned_model = pinned_printers.get(item.printer_id, ("", ""))[1]
|
||||
for other in items:
|
||||
if (
|
||||
other.id != item.id
|
||||
and other.id not in dispatch_ids
|
||||
and other.status == "pending"
|
||||
and other.printer_id == item.printer_id
|
||||
and _could_take_printer(other, item.printer_id, pinned_model)
|
||||
and not other.been_jumped
|
||||
and other.position < item.position
|
||||
and (
|
||||
@@ -2093,15 +2121,14 @@ class PrintScheduler:
|
||||
dispatch_ids.append(item.id)
|
||||
claim_printer(printer_id)
|
||||
|
||||
# SJF starvation guard: mark model-based items that were jumped
|
||||
# SJF starvation guard: mark items that were jumped
|
||||
if sjf_enabled and item.print_time_seconds is not None:
|
||||
for other in items:
|
||||
if (
|
||||
other.id != item.id
|
||||
and other.id not in dispatch_ids
|
||||
and other.status == "pending"
|
||||
and other.printer_id is None
|
||||
and other.target_model
|
||||
and other.target_model.upper() == item.target_model.upper()
|
||||
and _could_take_printer(other, printer_id, item.target_model)
|
||||
and not other.been_jumped
|
||||
and other.position < item.position
|
||||
and (
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
"""The queue's position sequence.
|
||||
|
||||
Positions are one sequence across every pending item, not one per printer
|
||||
(#3200). The queue page lists and reorders pending items as a single list, and
|
||||
the scheduler dispatches in that order, so a pinned job and an "Any <model>" job
|
||||
compete for a printer by position. A per-printer sequence gave each lane its
|
||||
own 1, 2, 3 and made a job added last land in the middle of the list.
|
||||
"""
|
||||
|
||||
from sqlalchemy import func, select, text
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.app.models.print_queue import PrintQueueItem
|
||||
|
||||
# Advisory lock key for the shared sequence, in the 1625 namespace. The key
|
||||
# used to be the printer id, back when every printer had a sequence of its own.
|
||||
QUEUE_POSITION_LOCK_KEY = 0
|
||||
|
||||
|
||||
async def lock_queue_positions(db: AsyncSession) -> None:
|
||||
"""Serialize inserts that read MAX(position) (#1625-followup).
|
||||
|
||||
Two concurrent inserts into an empty queue would otherwise both read
|
||||
MAX(position) as 0 and land on position 1. The lock is transaction-scoped
|
||||
and released at commit/rollback. SQLite serializes writes implicitly and
|
||||
needs no equivalent.
|
||||
|
||||
The dialect is read from the session binding, not the ``is_sqlite()``
|
||||
helper: the test fixture binds a SQLite engine while
|
||||
``settings.database_url`` may still point at Postgres.
|
||||
"""
|
||||
if db.get_bind().dialect.name == "postgresql":
|
||||
await db.execute(text("SELECT pg_advisory_xact_lock(1625, :k)"), {"k": QUEUE_POSITION_LOCK_KEY})
|
||||
|
||||
|
||||
async def max_queue_position(db: AsyncSession) -> int:
|
||||
"""Highest position among pending items, or 0 for an empty queue."""
|
||||
result = await db.execute(select(func.max(PrintQueueItem.position)).where(PrintQueueItem.status == "pending"))
|
||||
return result.scalar() or 0
|
||||
|
||||
|
||||
async def next_queue_position(db: AsyncSession) -> int:
|
||||
"""Position for an item appended to the end of the queue, under the lock."""
|
||||
await lock_queue_positions(db)
|
||||
return await max_queue_position(db) + 1
|
||||
@@ -1045,13 +1045,11 @@ class VirtualPrinterInstance:
|
||||
# empty queue.
|
||||
from sqlalchemy import func, select as _sql_select
|
||||
|
||||
# One sequence across all pending items, not one per
|
||||
# printer (#3200): the plates go to the end of the queue.
|
||||
queue_scope = _sql_select(func.max(PrintQueueItem.position)).where(
|
||||
PrintQueueItem.status == "pending"
|
||||
)
|
||||
if self.target_printer_id is not None:
|
||||
queue_scope = queue_scope.where(PrintQueueItem.printer_id == self.target_printer_id)
|
||||
else:
|
||||
queue_scope = queue_scope.where(PrintQueueItem.printer_id.is_(None))
|
||||
try:
|
||||
max_pos_raw = (await db.execute(queue_scope)).scalar()
|
||||
max_pos = int(max_pos_raw) if max_pos_raw is not None else 0
|
||||
|
||||
Reference in New Issue
Block a user