Skip to content

Conversation

@jage
Copy link
Member

@jage jage commented Sep 17, 2025

This is a demoe to show how this could work, not expecting this branch to be merged but rather as inspiration for a "real" implementation if we like this.

Background:

Since there's no concept of ready or unacked messages in streams, it is hard to see if the consumers are replaying old messages and/or not keeping up with the inflow and retention policy of a stream.

This is a rough draft how it could look like, besides listing consumers with position and throughput in a table, we also visualize the position by drawing a progress bar with all consumers.

Screenshot 2025-09-17 at 10 01 14

Video showing two consumers starting at the first stream position.

Screen.Recording.2025-09-17.at.10.00.01.mov

This is a demonstration to communicate how one way how this could work,
not expected this branch to be merged but rather as inspiration for a
new implementation if we like this.

Background:

Since there's no concept of ready or unacked messages in streams, it is
hard to see if the consumers are replaying old messages and/or not
keeping up with the inflow and retention policy of a stream.

This is a rough draft how it could look like, besides listing consumers
with position and throughput in a table, we also visualize the position
by drawing a progress bar with all consumers.
@claude
Copy link

claude bot commented Sep 17, 2025

Claude finished @jage's task —— View job


Code Review in Progress

Todo List:

  • Read and analyze all 7 changed files
  • Review Crystal backend changes (consumer.cr, stream.cr, stream_consumer.cr, server.cr)
  • Review frontend changes (queue.js, main.css, queue.ecr)
  • Check for bugs, security issues, performance problems, missing error handling
  • Report findings

Analysis Complete

After reviewing all changes, I found several issues that need addressing:

Crystal Backend Issues:

  1. src/lavinmq/amqp/stream/stream.cr:102 - Missing synchronization around consumer offset access. The consumer_reading_positions method accesses consumer offsets without proper locking, which could lead to race conditions when consumers are being modified concurrently.

  2. src/lavinmq/amqp/stream/stream_consumer.cr:216 - The stream_offset field in details_tuple exposes internal offset state without validation. If offset becomes invalid due to stream truncation, this could return stale data.

JavaScript Issues:

  1. static/js/queue.js:297-303 - Division by zero potential. When streamSize is 0, the position percentage calculation could behave unexpectedly. While there's a check for streamSize > 0, the percentage calculation (position / streamSize) * 100 should handle edge cases better.

  2. static/js/queue.js:91-106 - Event listeners for hover are added without cleanup. If the consumer table is refreshed frequently, these event listeners could accumulate, leading to memory leaks.

  3. static/js/queue.js:256-258 - Hard-coded color array length. The getConsumerColor function uses modulo with a fixed array, but if more than 10 consumers exist, colors will repeat, potentially making it harder to distinguish consumers visually.

CSS Issues:

  1. static/main.css:2576 - Undefined CSS variable --color-bg-highlight is referenced but never defined in the CSS file, which will cause the highlight effect to not work properly.

@viktorerlingsson
Copy link
Member

This is very cool! Would like to see us implementing something like this in the future 👍

@snichme
Copy link
Member

snichme commented Oct 3, 2025

Might help this PR out #1331 introduces a separate view for streams

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.

4 participants