Skip to content

feat: message ack with atomic cleanup (story 1.8)#10

Merged
vieiralucas merged 5 commits intomainfrom
feat/1-8-message-acknowledgment
Feb 12, 2026
Merged

feat: message ack with atomic cleanup (story 1.8)#10
vieiralucas merged 5 commits intomainfrom
feat/1-8-message-acknowledgment

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented Feb 12, 2026

Summary

  • Implement handle_ack in scheduler: validates lease exists, parses expiry from lease value, finds message key by scanning messages CF, atomically deletes message + lease + lease_expiry via WriteBatch
  • Implement Ack gRPC RPC with UUID parsing and input validation
  • Add parse_expiry_from_lease_value() to keys module
  • Acking unknown or already-acked message returns NOT_FOUND (idempotent)

Test plan

  • Full lifecycle: enqueue -> lease -> ack -> verify message and lease deleted from storage
  • Ack unknown message returns MessageNotFound
  • Double ack returns MessageNotFound on second call (idempotent)
  • Ack without lease returns error
  • 48 tests pass, clippy clean, cargo fmt clean

Recreated after GitHub auto-closed the original PR #8


Summary by cubic

Adds message acknowledgment (Story 1.8). Ack atomically removes the message, lease, and lease expiry, is idempotent, and rejects corrupt lease values.

  • New Features

    • Scheduler: handle_ack validates lease, parses expiry, finds the message key, and deletes in one WriteBatch.
    • Server: Ack RPC with input checks, UUID parsing, and error mapping (MessageNotFound -> NOT_FOUND, CorruptData/Storage -> INTERNAL).
  • Bug Fixes

    • Validate lease value structure and propagate storage errors when scanning messages to prevent “ghost” acks.
    • Per-queue round-robin delivery and error logging on lease rollback.

Written for commit 2bd310c. Summary will update on new commits.

@vieiralucas vieiralucas force-pushed the feat/1-8-message-acknowledgment branch from 69e45f6 to c00fee8 Compare February 12, 2026 01:38
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 7 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="crates/fila-core/src/storage/keys.rs">

<violation number="1" location="crates/fila-core/src/storage/keys.rs:103">
P2: `parse_expiry_from_lease_value` accepts any buffer ≥ 8 bytes and ignores the length prefix/separator. This can treat corrupted lease values as valid and derive a wrong expiry, leaving the real `lease_expiry` entry orphaned. Consider validating the encoded length and separator before parsing the expiry.</violation>
</file>

<file name="crates/fila-core/src/broker/scheduler.rs">

<violation number="1" location="crates/fila-core/src/broker/scheduler.rs:253">
P2: `find_message_key` drops `list_messages` errors, so `handle_ack` will still delete the lease/expiry and return success even when message scanning failed, leaving the message in the queue to be redelivered. Propagate the storage error instead of treating it as “message not found.”</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread crates/fila-core/src/storage/keys.rs Outdated
Comment thread crates/fila-core/src/broker/scheduler.rs Outdated
Base automatically changed from feat/1-7-consumer-lease-message-delivery to main February 12, 2026 01:42
…d expiry (story 1.8)

add handle_ack to scheduler that validates lease existence, parses expiry
timestamp from lease value, finds the full message key by scanning messages
CF, and atomically deletes the message, lease, and lease_expiry entries via
WriteBatch. implement Ack grpc rpc with uuid parsing and input validation.
acking an unknown or already-acked message returns MessageNotFound.
@vieiralucas vieiralucas force-pushed the feat/1-8-message-acknowledgment branch from c00fee8 to e7a8f09 Compare February 12, 2026 01:43
parse_expiry_from_lease_value now validates the length prefix and
separator instead of blindly grabbing the last 8 bytes, preventing
wrong expiry derivation from corrupted values.

find_message_key now propagates storage errors instead of swallowing
them with .ok(), preventing ack from succeeding while leaving the
message in the queue for redelivery.
@vieiralucas vieiralucas merged commit 4c8254c into main Feb 12, 2026
5 checks passed
@vieiralucas vieiralucas deleted the feat/1-8-message-acknowledgment branch February 12, 2026 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant