From b6c4b91dff22150909df84f0914e8abacf7429c1 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 16 Apr 2024 17:14:41 +0200 Subject: [PATCH 01/72] WIP adds automatic offset tracking for streams --- src/lavinmq/amqp/queue/stream_queue.cr | 4 + .../amqp/queue/stream_queue_message_store.cr | 83 +++++++++++++- src/lavinmq/client/channel/stream_consumer.cr | 101 ++++++++++++++++++ src/lavinmq/mfile.cr | 9 ++ 4 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 src/lavinmq/client/channel/stream_consumer.cr diff --git a/src/lavinmq/amqp/queue/stream_queue.cr b/src/lavinmq/amqp/queue/stream_queue.cr index 5b803debe9..7716b7b792 100644 --- a/src/lavinmq/amqp/queue/stream_queue.cr +++ b/src/lavinmq/amqp/queue/stream_queue.cr @@ -79,6 +79,10 @@ module LavinMQ::AMQP end end + def save_offset_by_consumer_tag(consumer_tag : String, offset : Int64) : Nil + stream_queue_msg_store.save_offset_by_consumer_tag(consumer_tag, offset) + end + # yield the next message in the ready queue # returns true if a message was deliviered, false otherwise # if we encouncer an unrecoverable ReadError, close queue diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 2d4f7a0336..b0fd845e04 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -1,4 +1,5 @@ require "./stream_queue" +require "../stream_consumer" module LavinMQ::AMQP class StreamQueue < DurableQueue @@ -11,11 +12,18 @@ module LavinMQ::AMQP @segment_last_ts = Hash(UInt32, Int64).new(0i64) # used for max-age @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg + @consumer_offsets_hash : Hash(String, Int64) # used for consumer offsets + @consumer_offsets : MFile + @consumer_offset_path : String def initialize(*args, **kwargs) super + @last_offset = get_last_offset build_segment_indexes + @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") + @consumer_offsets = MFile.new(@consumer_offset_path, 5000) # TODO: size? + @consumer_offsets_hash = consumer_offsets_hash drop_overflow end @@ -35,13 +43,17 @@ module LavinMQ::AMQP # Used once when a consumer is started # Populates `segment` and `position` by iterating through segments # until `offset` is found - def find_offset(offset) : Tuple(Int64, UInt32, UInt32) + def find_offset(offset, tag = nil) : Tuple(Int64, UInt32, UInt32) raise ClosedError.new if @closed case offset - when "first", nil then offset_at(@segments.first_key, 4u32) + when "first" then offset_at(@segments.first_key, 4u32) when "last" then offset_at(@segments.last_key, 4u32) when "next" then last_offset_seg_pos when Time then find_offset_in_segments(offset) + when nil + consumer_last_offset = last_offset_by_consumer_tag(tag) || 0 + find_offset_in_segments(consumer_last_offset) + # offset_at(@segments.first_key, 4u32) # TODO does this need to be handled? when Int if offset > @last_offset last_offset_seg_pos @@ -108,7 +120,72 @@ module LavinMQ::AMQP seg end - def shift?(consumer : AMQP::StreamConsumer) : Envelope? + def last_offset_by_consumer_tag(consumer_tag) + begin + if @consumer_offsets_hash[consumer_tag] + pos = @consumer_offsets_hash[consumer_tag] + tx = @consumer_offsets.to_slice(pos, 8) + return IO::ByteFormat::SystemEndian.decode(Int64, tx) + end + rescue KeyError + end + end + + private def consumer_offsets_hash + hash = Hash(String, Int64).new + slice = @consumer_offsets.to_slice + more_to_read = true + i = 0_i64 + ctag_start = 0 + while more_to_read && slice.size > 0 + if slice[i] == 32 + ctag = String.new(slice[ctag_start..i-1]) + pos = i+1 + hash[ctag] = pos + ctag_start = pos + 8 + end + more_to_read = false if (i += 1) == slice.size-1 + end + hash + end + + def save_offset_by_consumer_tag(consumer_tag, new_offset) + pos = 0_i64 + begin + if pos = @consumer_offsets_hash[consumer_tag] + buf = uninitialized UInt8[8] + IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) + @consumer_offsets.write_at(pos, buf.to_slice) + end + rescue KeyError + write_new_ctag_to_file(consumer_tag, new_offset) + end + end + + def write_new_ctag_to_file(consumer_tag, new_offset) + # pos = @consumer_offsets.size + slice.size + # @consumer_offsets.write(slice + buf.to_slice) + # TODO this should work? + # instead of having to manually find the position + highest_pos = 0_i64 + @consumer_offsets_hash.each do |key, value| + if value > highest_pos + highest_pos = value + end + end + pos = highest_pos + pos += 8 unless pos == 0 + slice = "#{consumer_tag} ".to_slice + @consumer_offsets.write_at(pos, slice) + pos += slice.size + buf = uninitialized UInt8[8] + IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) + @consumer_offsets.write_at(pos, buf.to_slice) + @consumer_offsets_hash[consumer_tag] = pos + pos + end + + def shift?(consumer : StreamConsumer) : Envelope? raise ClosedError.new if @closed if env = shift_requeued(consumer.requeued) diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr new file mode 100644 index 0000000000..9aba5c96e9 --- /dev/null +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -0,0 +1,101 @@ +require "./consumer" +require "../../segment_position" + +module LavinMQ + class Client + class Channel + class StreamConsumer < LavinMQ::Client::Channel::Consumer + property offset : Int64 + property segment : UInt32 + property pos : UInt32 + getter requeued = Deque(SegmentPosition).new + + def initialize(@channel : Client::Channel, @queue : StreamQueue, frame : AMQP::Frame::Basic::Consume) + validate_preconditions(frame) + offset = frame.arguments["x-stream-offset"]? + @tag = frame.consumer_tag + @offset, @segment, @pos = stream_queue.find_offset(offset, @tag) + super + end + + private def validate_preconditions(frame) + if frame.exclusive + raise Error::PreconditionFailed.new("Stream consumers must not be exclusive") + end + if frame.no_ack + raise Error::PreconditionFailed.new("Stream consumers must acknowledge messages") + end + if @channel.prefetch_count.zero? + raise Error::PreconditionFailed.new("Stream consumers must have a prefetch limit") + end + unless @channel.global_prefetch_count.zero? + raise Error::PreconditionFailed.new("Stream consumers does not support global prefetch limit") + end + if frame.arguments.has_key? "x-priority" + raise Error::PreconditionFailed.new("x-priority not supported on stream queues") + end + case frame.arguments["x-stream-offset"]? + when Nil, Int, Time, "first", "next", "last" + else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") + end + end + + private def deliver_loop + i = 0 + loop do + wait_for_capacity + loop do + raise ClosedError.new if @closed + next if wait_for_queue_ready + next if wait_for_paused_queue + next if wait_for_flow + break + end + {% unless flag?(:release) %} + @log.debug { "Getting a new message" } + {% end %} + stream_queue.consume_get(self) do |env| + deliver(env.message, env.segment_position, env.redelivered) + end + Fiber.yield if (i &+= 1) % 32768 == 0 + end + rescue ex : ClosedError | Queue::ClosedError | Client::Channel::ClosedError | ::Channel::ClosedError + @log.debug { "deliver loop exiting: #{ex.inspect}" } + end + + private def wait_for_queue_ready + if @offset > stream_queue.last_offset && @requeued.empty? + @log.debug { "Waiting for queue not to be empty" } + select + when stream_queue.new_messages.receive + @log.debug { "Queue is not empty" } + when @has_requeued.receive + @log.debug { "Got a requeued message" } + when @notify_closed.receive + end + return true + end + end + + @has_requeued = ::Channel(Nil).new + + private def stream_queue : StreamQueue + @queue.as(StreamQueue) + end + + def ack(sp) + stream_queue.save_offset_by_consumer_tag(@tag, @offset) # TODO only if no x-stream-offset? + super + end + + def reject(sp, requeue : Bool) + super + if requeue + @requeued.push(sp) + @has_requeued.try_send? nil if @requeued.size == 1 + end + end + end + end + end +end diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 8101b7d5c3..7d420889f1 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -203,6 +203,15 @@ class MFile < IO @size = new_size end + def write_at(pos : Int64, slice : Bytes) : Nil + raise ClosedError.new if @closed + end_pos = pos + slice.size + @size = end_pos if end_pos > @size + raise IO::EOFError.new if end_pos > @capacity + slice.copy_to(buffer + pos, slice.size) + end + + def read(slice : Bytes) pos = @pos len = Math.min(slice.size, @size - pos) From 64408ae5bfc44b23035902d575edfc4fe0df0711 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 16 Apr 2024 17:14:46 +0200 Subject: [PATCH 02/72] specs --- spec/stream_queue_spec.cr | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 474580b479..c2cf038d71 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -328,6 +328,76 @@ describe LavinMQ::AMQP::StreamQueue do msg.body_io.to_s.should eq "msg with filter 2" end end + describe "Automatic consumer offset tracking" do + args = {"x-queue-type": "stream"} + + it "resumes from last offset on reconnect" do + queue_name = Random::Secure.hex + consumer_tag = "ctag-1" + offset = 10 + + # publish 10 messages + with_channel do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + 10.times { |i| q.publish "m#{i}" } + end + + # get 10 messages, offset should be tracked and saved + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + q.subscribe(no_ack: false, tag: consumer_tag) do |msg| + msgs.send msg + msg.ack + end + offset.times do + msgs.receive + end + end + sleep 1.second + + # consume again, should start from last offset automatically + msg_offset = 0 + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + q.publish "m10" + q.subscribe(no_ack: false, tag: consumer_tag) do |msg| + msgs.send msg + end + msg = msgs.receive + msg.body_io.to_s.should eq "m10" + msg_offset = msg.properties.headers.not_nil!["x-stream-offset"].as(Int64) + end + msg_offset.should eq 11 + end + + it "reads offset file on init" do + queue_name = Random::Secure.hex + vhost = Server.vhosts["/"] + offsets = [84_i64, Random.rand(Int64), Random.rand(Int64), Random.rand(Int64)] + tag_prefix = "ctag-" + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + q.publish "a" + end + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + end + msg_store.close + + sleep 0.1 + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.last_offset_by_consumer_tag(tag_prefix + i.to_s).should eq offset + end + msg_store.close end end end From be7121611d5777ffad0337998849fb2f781f7ddd Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 11:40:13 +0200 Subject: [PATCH 03/72] add specs for edge cases. refactor specs so they are more readable --- spec/stream_queue_spec.cr | 112 +++++++++++------- src/lavinmq/client/channel/stream_consumer.cr | 8 +- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index c2cf038d71..1b503bbb0c 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -1,7 +1,31 @@ require "./spec_helper" require "./../src/lavinmq/amqp/queue" -describe LavinMQ::AMQP::StreamQueue do +module StreamQueueSpecHelpers + def self.publish(queue_name, nr_of_messages) + args = {"x-queue-type": "stream"} + with_channel do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + nr_of_messages.times { |i| q.publish "m#{i}" } + end + end + + def self.consume_one(queue_name, c_tag, c_args = AMQP::Client::Arguments.new()) + args = {"x-queue-type": "stream"} + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + q.subscribe(no_ack: false, tag: c_tag, args: c_args) do |msg| + msgs.send msg + msg.ack + end + msgs.receive + end + end +end + +describe LavinMQ::StreamQueue do stream_queue_args = LavinMQ::AMQP::Table.new({"x-queue-type": "stream"}) describe "Consume" do @@ -329,68 +353,34 @@ describe LavinMQ::AMQP::StreamQueue do end end describe "Automatic consumer offset tracking" do - args = {"x-queue-type": "stream"} - it "resumes from last offset on reconnect" do queue_name = Random::Secure.hex - consumer_tag = "ctag-1" - offset = 10 + consumer_tag = Random::Secure.hex + offset = 3 - # publish 10 messages - with_channel do |ch| - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - 10.times { |i| q.publish "m#{i}" } - end + StreamQueueSpecHelpers.publish(queue_name, offset+1) - # get 10 messages, offset should be tracked and saved - with_channel do |ch| - ch.prefetch 1 - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - msgs = Channel(AMQP::Client::DeliverMessage).new - q.subscribe(no_ack: false, tag: consumer_tag) do |msg| - msgs.send msg - msg.ack - end - offset.times do - msgs.receive - end - end - sleep 1.second + offset.times { StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) } + sleep 0.1 # consume again, should start from last offset automatically - msg_offset = 0 - with_channel do |ch| - ch.prefetch 1 - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - msgs = Channel(AMQP::Client::DeliverMessage).new - q.publish "m10" - q.subscribe(no_ack: false, tag: consumer_tag) do |msg| - msgs.send msg - end - msg = msgs.receive - msg.body_io.to_s.should eq "m10" - msg_offset = msg.properties.headers.not_nil!["x-stream-offset"].as(Int64) - end - msg_offset.should eq 11 + msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) + msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq offset+1 end - it "reads offset file on init" do + it "reads offsets from file on init" do queue_name = Random::Secure.hex vhost = Server.vhosts["/"] offsets = [84_i64, Random.rand(Int64), Random.rand(Int64), Random.rand(Int64)] tag_prefix = "ctag-" - with_channel do |ch| - ch.prefetch 1 - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - q.publish "a" - end + StreamQueueSpecHelpers.publish(queue_name, 1) + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) end msg_store.close - sleep 0.1 msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) @@ -399,5 +389,37 @@ describe LavinMQ::AMQP::StreamQueue do end msg_store.close end + + it "does not track offset if x-stream-offset is set" do + queue_name = Random::Secure.hex + consumer_tag = Random::Secure.hex + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0}) + + StreamQueueSpecHelpers.publish(queue_name, 2) + msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) + msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + sleep 0.1 + + # should consume the same message again since tracking was not saved from last consume + msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) + msg_2.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + end + + it "should not use saved offset if x-stream-offset is set" do + queue_name = Random::Secure.hex + consumer_tag = Random::Secure.hex + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0}) + + StreamQueueSpecHelpers.publish(queue_name, 2) + + # get message without x-stream-offset, tracks offset + msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) + msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + sleep 0.1 + + # consume with x-stream-offset set, should consume the same message again + msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) + msg_2.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + end end end diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index 9aba5c96e9..2c05b1e3e7 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -9,6 +9,7 @@ module LavinMQ property segment : UInt32 property pos : UInt32 getter requeued = Deque(SegmentPosition).new + @track_offset = false def initialize(@channel : Client::Channel, @queue : StreamQueue, frame : AMQP::Frame::Basic::Consume) validate_preconditions(frame) @@ -35,7 +36,10 @@ module LavinMQ raise Error::PreconditionFailed.new("x-priority not supported on stream queues") end case frame.arguments["x-stream-offset"]? - when Nil, Int, Time, "first", "next", "last" + when Nil + @track_offset = true + when Int, Time, "first", "next", "last" + @track_offset = false else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") end end @@ -84,7 +88,7 @@ module LavinMQ end def ack(sp) - stream_queue.save_offset_by_consumer_tag(@tag, @offset) # TODO only if no x-stream-offset? + stream_queue.save_offset_by_consumer_tag(@tag, @offset) if @track_offset super end From c8f9275854c1e7ee48c408597e77103ed803d483 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 12:00:46 +0200 Subject: [PATCH 04/72] format --- spec/stream_queue_spec.cr | 6 +++--- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 12 ++++++------ src/lavinmq/mfile.cr | 1 - 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 1b503bbb0c..20ca8e730c 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -10,7 +10,7 @@ module StreamQueueSpecHelpers end end - def self.consume_one(queue_name, c_tag, c_args = AMQP::Client::Arguments.new()) + def self.consume_one(queue_name, c_tag, c_args = AMQP::Client::Arguments.new) args = {"x-queue-type": "stream"} with_channel do |ch| ch.prefetch 1 @@ -358,14 +358,14 @@ describe LavinMQ::StreamQueue do consumer_tag = Random::Secure.hex offset = 3 - StreamQueueSpecHelpers.publish(queue_name, offset+1) + StreamQueueSpecHelpers.publish(queue_name, offset + 1) offset.times { StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) } sleep 0.1 # consume again, should start from last offset automatically msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq offset+1 + msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq offset + 1 end it "reads offsets from file on init" do diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index b0fd845e04..81f74fb20b 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -47,9 +47,9 @@ module LavinMQ::AMQP raise ClosedError.new if @closed case offset when "first" then offset_at(@segments.first_key, 4u32) - when "last" then offset_at(@segments.last_key, 4u32) - when "next" then last_offset_seg_pos - when Time then find_offset_in_segments(offset) + when "last" then offset_at(@segments.last_key, 4u32) + when "next" then last_offset_seg_pos + when Time then find_offset_in_segments(offset) when nil consumer_last_offset = last_offset_by_consumer_tag(tag) || 0 find_offset_in_segments(consumer_last_offset) @@ -139,12 +139,12 @@ module LavinMQ::AMQP ctag_start = 0 while more_to_read && slice.size > 0 if slice[i] == 32 - ctag = String.new(slice[ctag_start..i-1]) - pos = i+1 + ctag = String.new(slice[ctag_start..i - 1]) + pos = i + 1 hash[ctag] = pos ctag_start = pos + 8 end - more_to_read = false if (i += 1) == slice.size-1 + more_to_read = false if (i += 1) == slice.size - 1 end hash end diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 7d420889f1..15da20fc87 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -211,7 +211,6 @@ class MFile < IO slice.copy_to(buffer + pos, slice.size) end - def read(slice : Bytes) pos = @pos len = Math.min(slice.size, @size - pos) From 971617500a7a9bab5149bd334bc48db92aba6d23 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 15:03:38 +0200 Subject: [PATCH 05/72] refactor, dont init consumer tracking stuff unless needed. resize file on init to facilitate simple write. --- .../amqp/queue/stream_queue_message_store.cr | 60 ++++++++----------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 81f74fb20b..f567785eb7 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -12,18 +12,16 @@ module LavinMQ::AMQP @segment_last_ts = Hash(UInt32, Int64).new(0i64) # used for max-age @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg - @consumer_offsets_hash : Hash(String, Int64) # used for consumer offsets - @consumer_offsets : MFile @consumer_offset_path : String + @consumer_offset_positions : Hash(String, Int64)? # used for consumer offsets + @consumer_offsets : MFile? def initialize(*args, **kwargs) super - @last_offset = get_last_offset build_segment_indexes @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") @consumer_offsets = MFile.new(@consumer_offset_path, 5000) # TODO: size? - @consumer_offsets_hash = consumer_offsets_hash drop_overflow end @@ -122,40 +120,48 @@ module LavinMQ::AMQP def last_offset_by_consumer_tag(consumer_tag) begin - if @consumer_offsets_hash[consumer_tag] - pos = @consumer_offsets_hash[consumer_tag] - tx = @consumer_offsets.to_slice(pos, 8) + if consumer_offset_positions[consumer_tag] + pos = consumer_offset_positions[consumer_tag] + tx = consumer_offsets.to_slice(pos, 8) return IO::ByteFormat::SystemEndian.decode(Int64, tx) end rescue KeyError end end - private def consumer_offsets_hash - hash = Hash(String, Int64).new - slice = @consumer_offsets.to_slice + def consumer_offsets : MFile + return @consumer_offsets.not_nil! if @consumer_offsets + path = File.join(@queue_data_dir, "consumer_offsets") + @consumer_offsets = MFile.new(path, 5000) # TODO: size? + end + + private def consumer_offset_positions + return @consumer_offset_positions.not_nil! if @consumer_offset_positions + positions = Hash(String, Int64).new + slice = consumer_offsets.to_slice more_to_read = true i = 0_i64 ctag_start = 0 - while more_to_read && slice.size > 0 - if slice[i] == 32 + + slice.each_with_index do |byte, i| + if byte == 32 # if space ctag = String.new(slice[ctag_start..i - 1]) pos = i + 1 - hash[ctag] = pos + positions[ctag] = pos ctag_start = pos + 8 end - more_to_read = false if (i += 1) == slice.size - 1 end - hash + consumer_offsets.resize(ctag_start) # resize mfile to remove any empty bytes + @consumer_offset_positions = positions end def save_offset_by_consumer_tag(consumer_tag, new_offset) pos = 0_i64 begin - if pos = @consumer_offsets_hash[consumer_tag] + if pos = consumer_offset_positions[consumer_tag] buf = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) - @consumer_offsets.write_at(pos, buf.to_slice) + consumer_offsets.write_at(pos, buf.to_slice) end rescue KeyError write_new_ctag_to_file(consumer_tag, new_offset) @@ -163,26 +169,12 @@ module LavinMQ::AMQP end def write_new_ctag_to_file(consumer_tag, new_offset) - # pos = @consumer_offsets.size + slice.size - # @consumer_offsets.write(slice + buf.to_slice) - # TODO this should work? - # instead of having to manually find the position - highest_pos = 0_i64 - @consumer_offsets_hash.each do |key, value| - if value > highest_pos - highest_pos = value - end - end - pos = highest_pos - pos += 8 unless pos == 0 slice = "#{consumer_tag} ".to_slice - @consumer_offsets.write_at(pos, slice) - pos += slice.size buf = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) - @consumer_offsets.write_at(pos, buf.to_slice) - @consumer_offsets_hash[consumer_tag] = pos - pos + pos = consumer_offsets.size + slice.size + consumer_offsets.write(slice + buf.to_slice) + consumer_offset_positions[consumer_tag] = pos end def shift?(consumer : StreamConsumer) : Envelope? From c5eab1875b4e274c8784aa588cf46796acf988e7 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 15:04:12 +0200 Subject: [PATCH 06/72] add spec that checks that only one entry is saved per consumer tag --- spec/stream_queue_spec.cr | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 20ca8e730c..ef3c664dcf 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -390,6 +390,28 @@ describe LavinMQ::StreamQueue do msg_store.close end + it "only saves one entry per consumer tag" do + queue_name = Random::Secure.hex + vhost = Server.vhosts["/"] + offsets = [84_i64, Random.rand(Int64), Random.rand(Int64), Random.rand(Int64)] + consumer_tag = "ctag-1" + StreamQueueSpecHelpers.publish(queue_name, 1) + + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each do |offset| + msg_store.save_offset_by_consumer_tag(consumer_tag, offset) + end + msg_store.close + sleep 0.1 + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last + msg_store.consumer_offsets.size.should eq 15 + + msg_store.close + end + it "does not track offset if x-stream-offset is set" do queue_name = Random::Secure.hex consumer_tag = Random::Secure.hex From 8e8290d51622b678b1701ec9b0bbdd05993ed40b Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 15:04:43 +0200 Subject: [PATCH 07/72] format --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index f567785eb7..d090400f81 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -13,7 +13,7 @@ module LavinMQ::AMQP @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg @consumer_offset_path : String - @consumer_offset_positions : Hash(String, Int64)? # used for consumer offsets + @consumer_offset_positions : Hash(String, Int64)? # used for consumer offsets @consumer_offsets : MFile? def initialize(*args, **kwargs) From 8a5745dca644f0ff2befb039e991ce5c6b9f2029 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 17 Apr 2024 16:40:16 +0200 Subject: [PATCH 08/72] lint --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index d090400f81..ed04dd53a1 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -119,14 +119,12 @@ module LavinMQ::AMQP end def last_offset_by_consumer_tag(consumer_tag) - begin - if consumer_offset_positions[consumer_tag] - pos = consumer_offset_positions[consumer_tag] - tx = consumer_offsets.to_slice(pos, 8) - return IO::ByteFormat::SystemEndian.decode(Int64, tx) - end - rescue KeyError + if consumer_offset_positions[consumer_tag] + pos = consumer_offset_positions[consumer_tag] + tx = consumer_offsets.to_slice(pos, 8) + return IO::ByteFormat::SystemEndian.decode(Int64, tx) end + rescue KeyError end def consumer_offsets : MFile From 65c4fc1bd73c7a752aa417eb741d33b8a7fce732 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 22 Apr 2024 11:40:34 +0200 Subject: [PATCH 09/72] add function to remove consumer tags from file --- .../amqp/queue/stream_queue_message_store.cr | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index ed04dd53a1..216bea78f0 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -128,8 +128,13 @@ module LavinMQ::AMQP end def consumer_offsets : MFile +<<<<<<< HEAD return @consumer_offsets.not_nil! if @consumer_offsets path = File.join(@queue_data_dir, "consumer_offsets") +======= + return @consumer_offsets.not_nil! if @consumer_offsets && !@consumer_offsets.not_nil!.closed? + path = File.join(@data_dir, "consumer_offsets") +>>>>>>> 92280a2b (add function to remove consumer tags from file) @consumer_offsets = MFile.new(path, 5000) # TODO: size? end @@ -175,7 +180,25 @@ module LavinMQ::AMQP consumer_offset_positions[consumer_tag] = pos end - def shift?(consumer : StreamConsumer) : Envelope? + def remove_consumer_tag_from_file(consumer_tag) + @consumer_offset_positions = consumer_offset_positions.reject! { |k, v| k == consumer_tag } + + offsets_to_save = Hash(String, Int64).new + consumer_offset_positions.each do |ctag, pos| + offset = last_offset_by_consumer_tag(ctag) + next unless offset + offsets_to_save[ctag] = offset + end + + consumer_offsets.close + consumer_offsets.delete + offsets_to_save.each do |ctag, offset| + write_new_ctag_to_file(ctag, offset) + end + + end + + def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? raise ClosedError.new if @closed if env = shift_requeued(consumer.requeued) From 4055f8f9efc7ffa0e80565389c9e45ca4ab76ad5 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 22 Apr 2024 11:40:45 +0200 Subject: [PATCH 10/72] spec for removing consumer tags --- spec/stream_queue_spec.cr | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index ef3c664dcf..8b5a1e93ee 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -443,5 +443,27 @@ describe LavinMQ::StreamQueue do msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) msg_2.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 end + + it "removes offset" do + queue_name = Random::Secure.hex + vhost = Server.vhosts["/"] + offsets = [84_i64, 10_i64] + tag_prefix = "ctag-" + StreamQueueSpecHelpers.publish(queue_name, 1) + + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + end + sleep 0.1 + msg_store.remove_consumer_tag_from_file(tag_prefix + 1.to_s) + msg_store.close + sleep 0.1 + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil + msg_store.close + end end end From 76d0d973a8a639925fb5d59c625801db0cfadd74 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 22 Apr 2024 12:29:37 +0200 Subject: [PATCH 11/72] save length of ctag in file, dont use space as deliminator --- .../amqp/queue/stream_queue_message_store.cr | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 216bea78f0..76583751a8 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -142,19 +142,20 @@ module LavinMQ::AMQP return @consumer_offset_positions.not_nil! if @consumer_offset_positions positions = Hash(String, Int64).new slice = consumer_offsets.to_slice - more_to_read = true - i = 0_i64 - ctag_start = 0 - - slice.each_with_index do |byte, i| - if byte == 32 # if space - ctag = String.new(slice[ctag_start..i - 1]) - pos = i + 1 - positions[ctag] = pos - ctag_start = pos + 8 - end + return positions if slice.size.zero? + pos = 0 + + loop do + ctag_length = IO::ByteFormat::LittleEndian.decode(UInt8, slice[pos, pos+1]) + break if ctag_length == 0 + pos += 1 + ctag = String.new(slice[pos..pos+ctag_length-1]) + pos += ctag_length + positions[ctag] = pos + pos += 8 + break if pos >= slice.size end - consumer_offsets.resize(ctag_start) # resize mfile to remove any empty bytes + consumer_offsets.resize(pos) # resize mfile to remove any empty bytes @consumer_offset_positions = positions end @@ -171,12 +172,19 @@ module LavinMQ::AMQP end end + # should we write a null byte after each offset? def write_new_ctag_to_file(consumer_tag, new_offset) - slice = "#{consumer_tag} ".to_slice - buf = uninitialized UInt8[8] - IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) - pos = consumer_offsets.size + slice.size - consumer_offsets.write(slice + buf.to_slice) + slice = consumer_tag.to_slice + consumer_tag_length = slice.size.to_u8 + pos = consumer_offsets.size + slice.size + 1 + + length_buffer = uninitialized UInt8[1] + IO::ByteFormat::LittleEndian.encode(consumer_tag_length, length_buffer.to_slice) + + offset_buffer = uninitialized UInt8[8] + IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), offset_buffer.to_slice) + + consumer_offsets.write(length_buffer.to_slice + slice + offset_buffer.to_slice) consumer_offset_positions[consumer_tag] = pos end From c8fbc9aeb3bb09b2998dcc481911beee3a175c88 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 22 Apr 2024 12:30:13 +0200 Subject: [PATCH 12/72] format --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 76583751a8..3d20bd3d0b 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -146,10 +146,10 @@ module LavinMQ::AMQP pos = 0 loop do - ctag_length = IO::ByteFormat::LittleEndian.decode(UInt8, slice[pos, pos+1]) + ctag_length = IO::ByteFormat::LittleEndian.decode(UInt8, slice[pos, pos + 1]) break if ctag_length == 0 pos += 1 - ctag = String.new(slice[pos..pos+ctag_length-1]) + ctag = String.new(slice[pos..pos + ctag_length - 1]) pos += ctag_length positions[ctag] = pos pos += 8 @@ -203,7 +203,6 @@ module LavinMQ::AMQP offsets_to_save.each do |ctag, offset| write_new_ctag_to_file(ctag, offset) end - end def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? From b6bb89a895fb29206ade49d8c3068a3036f3f09f Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 22 Apr 2024 12:37:17 +0200 Subject: [PATCH 13/72] lint --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 3d20bd3d0b..6cc0fcf345 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -189,10 +189,10 @@ module LavinMQ::AMQP end def remove_consumer_tag_from_file(consumer_tag) - @consumer_offset_positions = consumer_offset_positions.reject! { |k, v| k == consumer_tag } + @consumer_offset_positions = consumer_offset_positions.reject! { |k, _v| k == consumer_tag } offsets_to_save = Hash(String, Int64).new - consumer_offset_positions.each do |ctag, pos| + consumer_offset_positions.each do |ctag, _p| offset = last_offset_by_consumer_tag(ctag) next unless offset offsets_to_save[ctag] = offset From 52919a34e28488e06acea54ee30ef88852610eab Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 23 Apr 2024 12:02:14 +0200 Subject: [PATCH 14/72] remove methods, use instance variables directly. add cleanup function --- .../amqp/queue/stream_queue_message_store.cr | 57 +++++++++++++------ 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 6cc0fcf345..144b7f0cda 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -22,6 +22,7 @@ module LavinMQ::AMQP build_segment_indexes @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") @consumer_offsets = MFile.new(@consumer_offset_path, 5000) # TODO: size? + @consumer_offset_positions = consumer_offset_positions drop_overflow end @@ -119,14 +120,14 @@ module LavinMQ::AMQP end def last_offset_by_consumer_tag(consumer_tag) - if consumer_offset_positions[consumer_tag] - pos = consumer_offset_positions[consumer_tag] - tx = consumer_offsets.to_slice(pos, 8) + if pos = @consumer_offset_positions[consumer_tag] + tx = @consumer_offsets.to_slice(pos, 8) return IO::ByteFormat::SystemEndian.decode(Int64, tx) end rescue KeyError end +<<<<<<< HEAD def consumer_offsets : MFile <<<<<<< HEAD return @consumer_offsets.not_nil! if @consumer_offsets @@ -138,10 +139,11 @@ module LavinMQ::AMQP @consumer_offsets = MFile.new(path, 5000) # TODO: size? end +======= +>>>>>>> 7127e6ea (remove methods, use instance variables directly. add cleanup function) private def consumer_offset_positions - return @consumer_offset_positions.not_nil! if @consumer_offset_positions positions = Hash(String, Int64).new - slice = consumer_offsets.to_slice + slice = @consumer_offsets.to_slice return positions if slice.size.zero? pos = 0 @@ -155,17 +157,17 @@ module LavinMQ::AMQP pos += 8 break if pos >= slice.size end - consumer_offsets.resize(pos) # resize mfile to remove any empty bytes - @consumer_offset_positions = positions + @consumer_offsets.resize(pos) # resize mfile to remove any empty bytes + positions end def save_offset_by_consumer_tag(consumer_tag, new_offset) pos = 0_i64 begin - if pos = consumer_offset_positions[consumer_tag] + if pos = @consumer_offset_positions[consumer_tag] buf = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) - consumer_offsets.write_at(pos, buf.to_slice) + @consumer_offsets.write_at(pos, buf.to_slice) end rescue KeyError write_new_ctag_to_file(consumer_tag, new_offset) @@ -176,7 +178,7 @@ module LavinMQ::AMQP def write_new_ctag_to_file(consumer_tag, new_offset) slice = consumer_tag.to_slice consumer_tag_length = slice.size.to_u8 - pos = consumer_offsets.size + slice.size + 1 + pos = @consumer_offsets.size + slice.size + 1 length_buffer = uninitialized UInt8[1] IO::ByteFormat::LittleEndian.encode(consumer_tag_length, length_buffer.to_slice) @@ -184,27 +186,50 @@ module LavinMQ::AMQP offset_buffer = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), offset_buffer.to_slice) - consumer_offsets.write(length_buffer.to_slice + slice + offset_buffer.to_slice) - consumer_offset_positions[consumer_tag] = pos + @consumer_offsets.write(length_buffer.to_slice + slice + offset_buffer.to_slice) + @consumer_offset_positions[consumer_tag] = pos + end + + def cleanup_consumer_offsets + offsets_to_save = Hash(String, Int64).new + lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) # handle + @consumer_offset_positions.each do |ctag, pos| + offset = last_offset_by_consumer_tag(ctag).not_nil! + next if offset < lowest_offset_in_stream + # Other scenarios to remove? + offsets_to_save[ctag] = offset + end + + delete_and_reopen_offsets_file + @consumer_offset_positions = Hash(String, Int64).new + offsets_to_save.each do |ctag, offset| + write_new_ctag_to_file(ctag, offset) + end end def remove_consumer_tag_from_file(consumer_tag) - @consumer_offset_positions = consumer_offset_positions.reject! { |k, _v| k == consumer_tag } + @consumer_offset_positions = @consumer_offset_positions.reject! { |k, _v| k == consumer_tag } offsets_to_save = Hash(String, Int64).new - consumer_offset_positions.each do |ctag, _p| + @consumer_offset_positions.each do |ctag, _p| offset = last_offset_by_consumer_tag(ctag) next unless offset offsets_to_save[ctag] = offset end - consumer_offsets.close - consumer_offsets.delete + delete_and_reopen_offsets_file offsets_to_save.each do |ctag, offset| write_new_ctag_to_file(ctag, offset) end end + def delete_and_reopen_offsets_file + @consumer_offsets.close + @consumer_offsets.delete + path = File.join(@data_dir, "consumer_offsets") + @consumer_offsets = MFile.new(path, 5000) # TODO: size? + end + def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? raise ClosedError.new if @closed From 589f868a46d664c5ad6f513d2fd3a2005307a426 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 23 Apr 2024 12:02:31 +0200 Subject: [PATCH 15/72] add spec for cleanup --- spec/stream_queue_spec.cr | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 8b5a1e93ee..73dacc3a1e 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -407,7 +407,7 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last - msg_store.consumer_offsets.size.should eq 15 + msg_store.@consumer_offsets.size.should eq 15 msg_store.close end @@ -463,6 +463,30 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil + msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] + msg_store.close + end + + it "cleanup_consumer_offsets removes outdated offset" do + queue_name = Random::Secure.hex + vhost = Server.vhosts["/"] + offsets = [84_i64, -10_i64] + tag_prefix = "ctag-" + StreamQueueSpecHelpers.publish(queue_name, 1) + + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + end + sleep 0.1 + msg_store.cleanup_consumer_offsets + msg_store.close + sleep 0.1 + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil + msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] msg_store.close end end From 77d9a38aa9d3ab821e5a45f03bd6bbd7aa4b96e9 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:32:08 +0200 Subject: [PATCH 16/72] raise before updating instance variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/mfile.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 15da20fc87..fdd172faad 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -206,8 +206,8 @@ class MFile < IO def write_at(pos : Int64, slice : Bytes) : Nil raise ClosedError.new if @closed end_pos = pos + slice.size - @size = end_pos if end_pos > @size raise IO::EOFError.new if end_pos > @capacity + @size = end_pos if end_pos > @size slice.copy_to(buffer + pos, slice.size) end From 8b29074eed143cd2b6d502fa71d572c214c3a483 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:34:25 +0200 Subject: [PATCH 17/72] refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 144b7f0cda..d90ff0a80d 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -120,11 +120,10 @@ module LavinMQ::AMQP end def last_offset_by_consumer_tag(consumer_tag) - if pos = @consumer_offset_positions[consumer_tag] + if pos = @consumer_offset_positions[consumer_tag]? tx = @consumer_offsets.to_slice(pos, 8) return IO::ByteFormat::SystemEndian.decode(Int64, tx) end - rescue KeyError end <<<<<<< HEAD From affa80083fecefddb905628688715d4908dbaac5 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:35:36 +0200 Subject: [PATCH 18/72] remove comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index d90ff0a80d..1849ff3e32 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -173,7 +173,6 @@ module LavinMQ::AMQP end end - # should we write a null byte after each offset? def write_new_ctag_to_file(consumer_tag, new_offset) slice = consumer_tag.to_slice consumer_tag_length = slice.size.to_u8 From 7307e77588ebfa765fea3969df5b7e62c6f28d6f Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:36:09 +0200 Subject: [PATCH 19/72] refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 1849ff3e32..e209f33ac5 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -163,7 +163,7 @@ module LavinMQ::AMQP def save_offset_by_consumer_tag(consumer_tag, new_offset) pos = 0_i64 begin - if pos = @consumer_offset_positions[consumer_tag] + if pos = @consumer_offset_positions[consumer_tag]? buf = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) @consumer_offsets.write_at(pos, buf.to_slice) From 451b8f9e155d025f1e00e9b1cd5e12713c4ff475 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:36:21 +0200 Subject: [PATCH 20/72] refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index e209f33ac5..318d011f08 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -167,9 +167,9 @@ module LavinMQ::AMQP buf = uninitialized UInt8[8] IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) @consumer_offsets.write_at(pos, buf.to_slice) + else + write_new_ctag_to_file(consumer_tag, new_offset) end - rescue KeyError - write_new_ctag_to_file(consumer_tag, new_offset) end end From ff12ef7ee82807fbecdda548c0e6ff8b23de5bd2 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 01:38:39 +0200 Subject: [PATCH 21/72] format --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 318d011f08..8238d8840f 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -126,20 +126,12 @@ module LavinMQ::AMQP end end -<<<<<<< HEAD def consumer_offsets : MFile -<<<<<<< HEAD - return @consumer_offsets.not_nil! if @consumer_offsets - path = File.join(@queue_data_dir, "consumer_offsets") -======= return @consumer_offsets.not_nil! if @consumer_offsets && !@consumer_offsets.not_nil!.closed? path = File.join(@data_dir, "consumer_offsets") ->>>>>>> 92280a2b (add function to remove consumer tags from file) @consumer_offsets = MFile.new(path, 5000) # TODO: size? end -======= ->>>>>>> 7127e6ea (remove methods, use instance variables directly. add cleanup function) private def consumer_offset_positions positions = Hash(String, Int64).new slice = @consumer_offsets.to_slice From e66145bb108b7e75aa8c87d25b048775deae2357 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:08:30 +0200 Subject: [PATCH 22/72] set size to 32768 for offsets file, change path name --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 8238d8840f..77debe2d8a 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -13,15 +13,16 @@ module LavinMQ::AMQP @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg @consumer_offset_path : String - @consumer_offset_positions : Hash(String, Int64)? # used for consumer offsets @consumer_offsets : MFile? + @consumer_offset_positions = Hash(String, Int64).new # used for consumer offsets + @consumer_offset_capacity = 32_768 def initialize(*args, **kwargs) super @last_offset = get_last_offset build_segment_indexes @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") - @consumer_offsets = MFile.new(@consumer_offset_path, 5000) # TODO: size? + @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) @consumer_offset_positions = consumer_offset_positions drop_overflow end @@ -216,8 +217,7 @@ module LavinMQ::AMQP def delete_and_reopen_offsets_file @consumer_offsets.close @consumer_offsets.delete - path = File.join(@data_dir, "consumer_offsets") - @consumer_offsets = MFile.new(path, 5000) # TODO: size? + @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) end def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? From 1a8763ff86f595e3c2c88d2aa01c24dbbfdcf186 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:09:00 +0200 Subject: [PATCH 23/72] remove comment --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 77debe2d8a..b1f49b64bf 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -53,7 +53,6 @@ module LavinMQ::AMQP when nil consumer_last_offset = last_offset_by_consumer_tag(tag) || 0 find_offset_in_segments(consumer_last_offset) - # offset_at(@segments.first_key, 4u32) # TODO does this need to be handled? when Int if offset > @last_offset last_offset_seg_pos From d0f343a6fcc4a8d98aa58f5e5a850879e8df2cea Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:22:18 +0200 Subject: [PATCH 24/72] refactor restore_consumer_offset_positions --- .../amqp/queue/stream_queue_message_store.cr | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index b1f49b64bf..238e350f0e 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -23,7 +23,7 @@ module LavinMQ::AMQP build_segment_indexes @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) - @consumer_offset_positions = consumer_offset_positions + @consumer_offset_positions = restore_consumer_offset_positions drop_overflow end @@ -126,29 +126,20 @@ module LavinMQ::AMQP end end - def consumer_offsets : MFile - return @consumer_offsets.not_nil! if @consumer_offsets && !@consumer_offsets.not_nil!.closed? - path = File.join(@data_dir, "consumer_offsets") - @consumer_offsets = MFile.new(path, 5000) # TODO: size? - end - - private def consumer_offset_positions + private def restore_consumer_offset_positions positions = Hash(String, Int64).new - slice = @consumer_offsets.to_slice - return positions if slice.size.zero? - pos = 0 + return positions if @consumer_offsets.size.zero? loop do - ctag_length = IO::ByteFormat::LittleEndian.decode(UInt8, slice[pos, pos + 1]) - break if ctag_length == 0 - pos += 1 - ctag = String.new(slice[pos..pos + ctag_length - 1]) - pos += ctag_length - positions[ctag] = pos - pos += 8 - break if pos >= slice.size + ctag_length = @consumer_offsets.read_byte + break if !ctag_length || ctag_length.zero? + ctag = @consumer_offsets.read_string(ctag_length) + positions[ctag] = @consumer_offsets.pos + @consumer_offsets.skip(8) + rescue IO::EOFError + break end - @consumer_offsets.resize(pos) # resize mfile to remove any empty bytes + @consumer_offsets.resize(@consumer_offsets.pos) # resize mfile to remove any empty bytes positions end From e7d9592e7f89e4f018264243da9ee10f5d4a13c2 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:22:36 +0200 Subject: [PATCH 25/72] remove unused var --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 238e350f0e..add0aebfa2 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -144,7 +144,6 @@ module LavinMQ::AMQP end def save_offset_by_consumer_tag(consumer_tag, new_offset) - pos = 0_i64 begin if pos = @consumer_offset_positions[consumer_tag]? buf = uninitialized UInt8[8] From 9cb22bbfb35dd17b460353cad558f6bc4d0c6adb Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:24:19 +0200 Subject: [PATCH 26/72] save_offset_by_consumer_tag -> update_consumer_offset --- spec/stream_queue_spec.cr | 8 ++++---- src/lavinmq/amqp/queue/stream_queue.cr | 4 ++-- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- src/lavinmq/client/channel/stream_consumer.cr | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 73dacc3a1e..244068e345 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -378,7 +378,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| - msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) end msg_store.close sleep 0.1 @@ -400,7 +400,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each do |offset| - msg_store.save_offset_by_consumer_tag(consumer_tag, offset) + msg_store.update_consumer_offset(consumer_tag, offset) end msg_store.close sleep 0.1 @@ -454,7 +454,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| - msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) end sleep 0.1 msg_store.remove_consumer_tag_from_file(tag_prefix + 1.to_s) @@ -477,7 +477,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| - msg_store.save_offset_by_consumer_tag(tag_prefix + i.to_s, offset) + msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) end sleep 0.1 msg_store.cleanup_consumer_offsets diff --git a/src/lavinmq/amqp/queue/stream_queue.cr b/src/lavinmq/amqp/queue/stream_queue.cr index 7716b7b792..ee11039c88 100644 --- a/src/lavinmq/amqp/queue/stream_queue.cr +++ b/src/lavinmq/amqp/queue/stream_queue.cr @@ -79,8 +79,8 @@ module LavinMQ::AMQP end end - def save_offset_by_consumer_tag(consumer_tag : String, offset : Int64) : Nil - stream_queue_msg_store.save_offset_by_consumer_tag(consumer_tag, offset) + def update_consumer_offset(consumer_tag : String, offset : Int64) : Nil + stream_queue_msg_store.update_consumer_offset(consumer_tag, offset) end # yield the next message in the ready queue diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index add0aebfa2..0cd11553b0 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -143,7 +143,7 @@ module LavinMQ::AMQP positions end - def save_offset_by_consumer_tag(consumer_tag, new_offset) + def update_consumer_offset(consumer_tag : String, new_offset : Int64) begin if pos = @consumer_offset_positions[consumer_tag]? buf = uninitialized UInt8[8] diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index 2c05b1e3e7..da235f61a9 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -88,7 +88,7 @@ module LavinMQ end def ack(sp) - stream_queue.save_offset_by_consumer_tag(@tag, @offset) if @track_offset + stream_queue.update_consumer_offset(@tag, @offset) if @track_offset super end From b42be915af112ce70d618fd11e85d63d7838bd49 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:25:15 +0200 Subject: [PATCH 27/72] write_new_ctag_to_file -> store_consumer_offset --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 0cd11553b0..cf43572cc8 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -150,12 +150,12 @@ module LavinMQ::AMQP IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) @consumer_offsets.write_at(pos, buf.to_slice) else - write_new_ctag_to_file(consumer_tag, new_offset) + store_consumer_offset(consumer_tag, new_offset) end end end - def write_new_ctag_to_file(consumer_tag, new_offset) + def store_consumer_offset(consumer_tag, new_offset) slice = consumer_tag.to_slice consumer_tag_length = slice.size.to_u8 pos = @consumer_offsets.size + slice.size + 1 @@ -183,7 +183,7 @@ module LavinMQ::AMQP delete_and_reopen_offsets_file @consumer_offset_positions = Hash(String, Int64).new offsets_to_save.each do |ctag, offset| - write_new_ctag_to_file(ctag, offset) + store_consumer_offset(ctag, offset) end end @@ -199,7 +199,7 @@ module LavinMQ::AMQP delete_and_reopen_offsets_file offsets_to_save.each do |ctag, offset| - write_new_ctag_to_file(ctag, offset) + store_consumer_offset(ctag, offset) end end From f7cf45e90b0f05bbf3d4ee0f43fa23012d43a109 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 10:26:37 +0200 Subject: [PATCH 28/72] refactor update_consumer_offset --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index cf43572cc8..6cd91334f1 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -146,9 +146,8 @@ module LavinMQ::AMQP def update_consumer_offset(consumer_tag : String, new_offset : Int64) begin if pos = @consumer_offset_positions[consumer_tag]? - buf = uninitialized UInt8[8] - IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), buf.to_slice) - @consumer_offsets.write_at(pos, buf.to_slice) + @consumer_offsets.pos = pos + @consumer_offsets.write_bytes new_offset else store_consumer_offset(consumer_tag, new_offset) end From 74dc942dbbdf6a8c01a07cc00c55ac9bb132f8f5 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:11:29 +0200 Subject: [PATCH 29/72] add option to get a writeable slice from mfile --- src/lavinmq/mfile.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index fdd172faad..751e087aab 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -259,9 +259,9 @@ class MFile < IO Bytes.new(buffer, @size, read_only: true) end - def to_slice(pos, size) + def to_slice(pos, size, read_only = true) raise IO::EOFError.new if pos + size > @size - Bytes.new(buffer + pos, size, read_only: true) + Bytes.new(buffer + pos, size, read_only: read_only) end def advise(advice : Advice, addr = buffer, offset = 0, length = @capacity) : Nil From c0f10219c449af464c1eec19542b0ffaf791ef05 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:11:58 +0200 Subject: [PATCH 30/72] encode directly into mmap. fix bug with pos/size --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 6cd91334f1..b5c361d1b4 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -139,15 +139,15 @@ module LavinMQ::AMQP rescue IO::EOFError break end - @consumer_offsets.resize(@consumer_offsets.pos) # resize mfile to remove any empty bytes + @consumer_offsets.pos = 0 if @consumer_offsets.pos == 1 + @consumer_offsets.resize(@consumer_offsets.pos) positions end def update_consumer_offset(consumer_tag : String, new_offset : Int64) begin if pos = @consumer_offset_positions[consumer_tag]? - @consumer_offsets.pos = pos - @consumer_offsets.write_bytes new_offset + IO::ByteFormat::SystemEndian.encode(new_offset, @consumer_offsets.to_slice(pos, 8, false)) else store_consumer_offset(consumer_tag, new_offset) end From 148fe883bd9ac095bebf47df86329ff918d16daa Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:12:06 +0200 Subject: [PATCH 31/72] update spec --- spec/stream_queue_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 244068e345..f1ba11c1f1 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -407,7 +407,7 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last - msg_store.@consumer_offsets.size.should eq 15 + msg_store.@consumer_offsets.size.should eq 16 msg_store.close end From af6172fd9058526877f1f01628b902c368a354a6 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:16:09 +0200 Subject: [PATCH 32/72] more strict with types --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index b5c361d1b4..885be8f2da 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -126,7 +126,7 @@ module LavinMQ::AMQP end end - private def restore_consumer_offset_positions + private def restore_consumer_offset_positions : Hash(String, Int64) positions = Hash(String, Int64).new return positions if @consumer_offsets.size.zero? @@ -154,7 +154,7 @@ module LavinMQ::AMQP end end - def store_consumer_offset(consumer_tag, new_offset) + def store_consumer_offset(consumer_tag : String, new_offset : Int64) slice = consumer_tag.to_slice consumer_tag_length = slice.size.to_u8 pos = @consumer_offsets.size + slice.size + 1 @@ -186,7 +186,7 @@ module LavinMQ::AMQP end end - def remove_consumer_tag_from_file(consumer_tag) + def remove_consumer_tag_from_file(consumer_tag : String) @consumer_offset_positions = @consumer_offset_positions.reject! { |k, _v| k == consumer_tag } offsets_to_save = Hash(String, Int64).new From e16682d98b406d85e8c666fad3c1941a8867dd38 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:16:55 +0200 Subject: [PATCH 33/72] format --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 885be8f2da..03a2a5c544 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -13,7 +13,7 @@ module LavinMQ::AMQP @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg @consumer_offset_path : String - @consumer_offsets : MFile? + @consumer_offsets : MFile @consumer_offset_positions = Hash(String, Int64).new # used for consumer offsets @consumer_offset_capacity = 32_768 From 2567e4ca6834629e9cb0ab8829fc42a276e548cd Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:37:06 +0200 Subject: [PATCH 34/72] lint --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 03a2a5c544..a2d8852296 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -145,12 +145,10 @@ module LavinMQ::AMQP end def update_consumer_offset(consumer_tag : String, new_offset : Int64) - begin - if pos = @consumer_offset_positions[consumer_tag]? - IO::ByteFormat::SystemEndian.encode(new_offset, @consumer_offsets.to_slice(pos, 8, false)) - else - store_consumer_offset(consumer_tag, new_offset) - end + if pos = @consumer_offset_positions[consumer_tag]? + IO::ByteFormat::SystemEndian.encode(new_offset, @consumer_offsets.to_slice(pos, 8, false)) + else + store_consumer_offset(consumer_tag, new_offset) end end From 83d55369eadcdeaf62d720b4491a99cfab65b92b Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 24 Apr 2024 11:39:26 +0200 Subject: [PATCH 35/72] lint --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index a2d8852296..c36c1aa52f 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -169,11 +169,10 @@ module LavinMQ::AMQP def cleanup_consumer_offsets offsets_to_save = Hash(String, Int64).new - lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) # handle - @consumer_offset_positions.each do |ctag, pos| - offset = last_offset_by_consumer_tag(ctag).not_nil! - next if offset < lowest_offset_in_stream - # Other scenarios to remove? + lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) + @consumer_offset_positions.each do |ctag, _pos| + offset = last_offset_by_consumer_tag(ctag) + next if !offset || offset < lowest_offset_in_stream offsets_to_save[ctag] = offset end From e7ace220f732a49f4110615eb371f71cf0cf2903 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 13:59:41 +0200 Subject: [PATCH 36/72] String#bytesize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index c36c1aa52f..79d4cfdf6b 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -154,7 +154,7 @@ module LavinMQ::AMQP def store_consumer_offset(consumer_tag : String, new_offset : Int64) slice = consumer_tag.to_slice - consumer_tag_length = slice.size.to_u8 + consumer_tag_length = slice.bytesize.to_u8 pos = @consumer_offsets.size + slice.size + 1 length_buffer = uninitialized UInt8[1] From 85d9d39bb2d365d0cfbfcd881a766cf87eb9ec03 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 14:27:49 +0200 Subject: [PATCH 37/72] refactor store_consumer_offset and restore_consumer_offset_positions --- .../amqp/queue/stream_queue_message_store.cr | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 79d4cfdf6b..0049e2abd3 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -131,9 +131,8 @@ module LavinMQ::AMQP return positions if @consumer_offsets.size.zero? loop do - ctag_length = @consumer_offsets.read_byte - break if !ctag_length || ctag_length.zero? - ctag = @consumer_offsets.read_string(ctag_length) + ctag = AMQ::Protocol::ShortString.from_io(@consumer_offsets) + break if ctag.empty? positions[ctag] = @consumer_offsets.pos @consumer_offsets.skip(8) rescue IO::EOFError @@ -153,18 +152,9 @@ module LavinMQ::AMQP end def store_consumer_offset(consumer_tag : String, new_offset : Int64) - slice = consumer_tag.to_slice - consumer_tag_length = slice.bytesize.to_u8 - pos = @consumer_offsets.size + slice.size + 1 - - length_buffer = uninitialized UInt8[1] - IO::ByteFormat::LittleEndian.encode(consumer_tag_length, length_buffer.to_slice) - - offset_buffer = uninitialized UInt8[8] - IO::ByteFormat::LittleEndian.encode(new_offset.as(Int64), offset_buffer.to_slice) - - @consumer_offsets.write(length_buffer.to_slice + slice + offset_buffer.to_slice) - @consumer_offset_positions[consumer_tag] = pos + @consumer_offsets.write_bytes AMQ::Protocol::ShortString.new(consumer_tag) + @consumer_offset_positions[consumer_tag] = @consumer_offsets.size + @consumer_offsets.write_bytes new_offset end def cleanup_consumer_offsets From 9019d1ec0ef18d3389e6df98448cb81475075a39 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 14:40:10 +0200 Subject: [PATCH 38/72] refactor cleanup_consumer_offsets --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 0049e2abd3..09aa55fc5d 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -161,9 +161,9 @@ module LavinMQ::AMQP offsets_to_save = Hash(String, Int64).new lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) @consumer_offset_positions.each do |ctag, _pos| - offset = last_offset_by_consumer_tag(ctag) - next if !offset || offset < lowest_offset_in_stream - offsets_to_save[ctag] = offset + if offset = last_offset_by_consumer_tag(ctag) + offsets_to_save[ctag] = offset if offset > lowest_offset_in_stream + end end delete_and_reopen_offsets_file From 9c97f9b97a6de74ea19dd7e38819d13211bd2184 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 14:49:16 +0200 Subject: [PATCH 39/72] replace offset file instead of delete --- .../amqp/queue/stream_queue_message_store.cr | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 09aa55fc5d..537406860c 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -166,10 +166,11 @@ module LavinMQ::AMQP end end - delete_and_reopen_offsets_file @consumer_offset_positions = Hash(String, Int64).new - offsets_to_save.each do |ctag, offset| - store_consumer_offset(ctag, offset) + replace_offsets_file do + offsets_to_save.each do |ctag, offset| + store_consumer_offset(ctag, offset) + end end end @@ -183,16 +184,19 @@ module LavinMQ::AMQP offsets_to_save[ctag] = offset end - delete_and_reopen_offsets_file - offsets_to_save.each do |ctag, offset| - store_consumer_offset(ctag, offset) + replace_offsets_file do + offsets_to_save.each do |ctag, offset| + store_consumer_offset(ctag, offset) + end end end - def delete_and_reopen_offsets_file - @consumer_offsets.close - @consumer_offsets.delete - @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) + def replace_offsets_file(&) + old_consumer_offsets = @consumer_offsets + @consumer_offsets = MFile.new("#{@consumer_offset_path}.tmp", @consumer_offset_capacity) + yield # fill the new file with correct data in this block + File.rename "#{@consumer_offset_path}.tmp", @consumer_offset_path + old_consumer_offsets.close end def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? From a22ec0759276649d8953fcee4d7793cf0438dcdf Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 15:03:49 +0200 Subject: [PATCH 40/72] lint --- spec/stream_queue_spec.cr | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index f1ba11c1f1..d40266c93c 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -23,6 +23,13 @@ module StreamQueueSpecHelpers msgs.receive end end + + def self.offset_from_headers(headers) + if headers + headers["x-stream-offset"].as(Int64) + else fail("No headers found") + end + end end describe LavinMQ::StreamQueue do @@ -365,7 +372,7 @@ describe LavinMQ::StreamQueue do # consume again, should start from last offset automatically msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq offset + 1 + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq offset + 1 end it "reads offsets from file on init" do @@ -419,12 +426,12 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.publish(queue_name, 2) msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 sleep 0.1 # should consume the same message again since tracking was not saved from last consume msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - msg_2.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 end it "should not use saved offset if x-stream-offset is set" do @@ -436,12 +443,12 @@ describe LavinMQ::StreamQueue do # get message without x-stream-offset, tracks offset msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - msg.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 sleep 0.1 # consume with x-stream-offset set, should consume the same message again msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - msg_2.properties.headers.not_nil!["x-stream-offset"].as(Int64).should eq 1 + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 end it "removes offset" do From 1a443f37a30f2b1d6ff3bcfbb4d714159bc88601 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 16 May 2024 15:15:00 +0200 Subject: [PATCH 41/72] lint --- spec/stream_queue_spec.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index d40266c93c..262cd3af6b 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -27,7 +27,8 @@ module StreamQueueSpecHelpers def self.offset_from_headers(headers) if headers headers["x-stream-offset"].as(Int64) - else fail("No headers found") + else + fail("No headers found") end end end From 778d299e50c2078bd8a6fdf4c1d5d459d6ba061d Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 23 May 2024 16:38:14 +0200 Subject: [PATCH 42/72] dont track offsets when consumer tag is generated --- spec/stream_queue_spec.cr | 23 +++++++++++++++++++ src/lavinmq/client/channel/stream_consumer.cr | 4 ++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 262cd3af6b..4dfa0a2f67 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -497,5 +497,28 @@ describe LavinMQ::StreamQueue do msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] msg_store.close end + + it "does not track offset if c-tag is auto-generated" do + queue_name = Random::Secure.hex + StreamQueueSpecHelpers.publish(queue_name, 1) + args = {"x-queue-type": "stream"} + c_tag = "" + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + c_tag = q.subscribe(no_ack: false) do |msg| + msgs.send msg + msg.ack + end + msg = msgs.receive + end + + sleep 0.1 + vhost = Server.vhosts["/"] + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(c_tag).should eq nil + end end end diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index da235f61a9..0a44a2a9b3 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -12,9 +12,9 @@ module LavinMQ @track_offset = false def initialize(@channel : Client::Channel, @queue : StreamQueue, frame : AMQP::Frame::Basic::Consume) + @tag = frame.consumer_tag validate_preconditions(frame) offset = frame.arguments["x-stream-offset"]? - @tag = frame.consumer_tag @offset, @segment, @pos = stream_queue.find_offset(offset, @tag) super end @@ -37,7 +37,7 @@ module LavinMQ end case frame.arguments["x-stream-offset"]? when Nil - @track_offset = true + @track_offset = true unless @tag.starts_with?("amq.ctag-") when Int, Time, "first", "next", "last" @track_offset = false else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") From d7bc4347612bfe5d1c740de0144d3883537a824c Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 23 May 2024 16:44:56 +0200 Subject: [PATCH 43/72] remove unused code --- spec/stream_queue_spec.cr | 23 ------------------- .../amqp/queue/stream_queue_message_store.cr | 17 -------------- 2 files changed, 40 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 4dfa0a2f67..4f43d22ce4 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -452,29 +452,6 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 end - it "removes offset" do - queue_name = Random::Secure.hex - vhost = Server.vhosts["/"] - offsets = [84_i64, 10_i64] - tag_prefix = "ctag-" - StreamQueueSpecHelpers.publish(queue_name, 1) - - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - offsets.each_with_index do |offset, i| - msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) - end - sleep 0.1 - msg_store.remove_consumer_tag_from_file(tag_prefix + 1.to_s) - msg_store.close - sleep 0.1 - - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil - msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] - msg_store.close - end - it "cleanup_consumer_offsets removes outdated offset" do queue_name = Random::Secure.hex vhost = Server.vhosts["/"] diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 537406860c..292c0f85df 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -174,23 +174,6 @@ module LavinMQ::AMQP end end - def remove_consumer_tag_from_file(consumer_tag : String) - @consumer_offset_positions = @consumer_offset_positions.reject! { |k, _v| k == consumer_tag } - - offsets_to_save = Hash(String, Int64).new - @consumer_offset_positions.each do |ctag, _p| - offset = last_offset_by_consumer_tag(ctag) - next unless offset - offsets_to_save[ctag] = offset - end - - replace_offsets_file do - offsets_to_save.each do |ctag, offset| - store_consumer_offset(ctag, offset) - end - end - end - def replace_offsets_file(&) old_consumer_offsets = @consumer_offsets @consumer_offsets = MFile.new("#{@consumer_offset_path}.tmp", @consumer_offset_capacity) From 15564b62363e5aeea55bae5348265557d3ae3672 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 28 May 2024 16:42:26 +0200 Subject: [PATCH 44/72] cleanup consumer offsets when dropping overflow --- spec/stream_queue_spec.cr | 47 +++++++++++++++++-- .../amqp/queue/stream_queue_message_store.cr | 5 +- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 4f43d22ce4..3f3d15fcae 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -379,7 +379,7 @@ describe LavinMQ::StreamQueue do it "reads offsets from file on init" do queue_name = Random::Secure.hex vhost = Server.vhosts["/"] - offsets = [84_i64, Random.rand(Int64), Random.rand(Int64), Random.rand(Int64)] + offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] tag_prefix = "ctag-" StreamQueueSpecHelpers.publish(queue_name, 1) @@ -401,7 +401,7 @@ describe LavinMQ::StreamQueue do it "only saves one entry per consumer tag" do queue_name = Random::Secure.hex vhost = Server.vhosts["/"] - offsets = [84_i64, Random.rand(Int64), Random.rand(Int64), Random.rand(Int64)] + offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] consumer_tag = "ctag-1" StreamQueueSpecHelpers.publish(queue_name, 1) @@ -415,7 +415,7 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last - msg_store.@consumer_offsets.size.should eq 16 + msg_store.@consumer_offsets.size.should eq 15 msg_store.close end @@ -475,6 +475,47 @@ describe LavinMQ::StreamQueue do msg_store.close end + it "runs cleanup when removing segment" do + consumer_tag = "ctag-1" + offset = -1 + vhost = Server.vhosts["/"] + queue_name = Random::Secure.hex + args = {"x-queue-type": "stream", "x-max-length": 2} + body_size = 256 * 1024 + data = Bytes.new(body_size) + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + + with_channel do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + q.publish_confirm data + end + sleep 0.1 + + with_channel do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + q.subscribe(no_ack: false, tag: consumer_tag) do |msg| + msgs.send msg + msg.ack + end + msgs.receive + end + + sleep 0.1 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq 2 + + with_channel do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + 4.times { q.publish_confirm data } + end + sleep 0.1 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil + + end + it "does not track offset if c-tag is auto-generated" do queue_name = Random::Secure.hex StreamQueueSpecHelpers.publish(queue_name, 1) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 292c0f85df..f41b68b6c6 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -158,11 +158,13 @@ module LavinMQ::AMQP end def cleanup_consumer_offsets + return if @consumer_offsets.size.zero? + offsets_to_save = Hash(String, Int64).new lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) @consumer_offset_positions.each do |ctag, _pos| if offset = last_offset_by_consumer_tag(ctag) - offsets_to_save[ctag] = offset if offset > lowest_offset_in_stream + offsets_to_save[ctag] = offset if offset >= lowest_offset_in_stream end end @@ -265,6 +267,7 @@ module LavinMQ::AMQP Time.unix_ms(last_ts) < min_ts end end + cleanup_consumer_offsets end private def drop_segments_while(& : UInt32 -> Bool) From dcb855684a37246e2623d622b70f934a520938fb Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 28 May 2024 16:43:32 +0200 Subject: [PATCH 45/72] format --- spec/stream_queue_spec.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 3f3d15fcae..7203d1aa12 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -513,7 +513,6 @@ describe LavinMQ::StreamQueue do sleep 0.1 msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil - end it "does not track offset if c-tag is auto-generated" do From 9face3814cd37ac9a862b41b3417217453622b69 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 28 May 2024 16:44:56 +0200 Subject: [PATCH 46/72] lint --- spec/stream_queue_spec.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 7203d1aa12..d492480501 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -477,7 +477,6 @@ describe LavinMQ::StreamQueue do it "runs cleanup when removing segment" do consumer_tag = "ctag-1" - offset = -1 vhost = Server.vhosts["/"] queue_name = Random::Secure.hex args = {"x-queue-type": "stream", "x-max-length": 2} @@ -528,7 +527,7 @@ describe LavinMQ::StreamQueue do msgs.send msg msg.ack end - msg = msgs.receive + msgs.receive end sleep 0.1 From f465889a7a0d28e439059cf4b776554fa8ad7f85 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 30 May 2024 15:22:56 +0200 Subject: [PATCH 47/72] handle large messages causing first segment to be empty --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index f41b68b6c6..379f465ed8 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -63,12 +63,15 @@ module LavinMQ::AMQP end end - private def offset_at(seg, pos) : Tuple(Int64, UInt32, UInt32) + private def offset_at(seg, pos, retried = false) : Tuple(Int64, UInt32, UInt32) return {@last_offset, seg, pos} if @size.zero? mfile = @segments[seg] msg = BytesMessage.from_bytes(mfile.to_slice + pos) offset = offset_from_headers(msg.properties.headers) {offset, seg, pos} + rescue ex : IndexError # first segment can be empty if message size >= segment size + return offset_at(seg + 1, pos, true) unless retried + raise ex end private def last_offset_seg_pos From 724ded7882f6892226fd3970a4e527f79844aa65 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 30 May 2024 15:23:12 +0200 Subject: [PATCH 48/72] cleanup spec --- spec/stream_queue_spec.cr | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index d492480501..9e29d8f759 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -479,16 +479,14 @@ describe LavinMQ::StreamQueue do consumer_tag = "ctag-1" vhost = Server.vhosts["/"] queue_name = Random::Secure.hex - args = {"x-queue-type": "stream", "x-max-length": 2} - body_size = 256 * 1024 - data = Bytes.new(body_size) + args = {"x-queue-type": "stream", "x-max-length": 1} + msg_body = Bytes.new(LavinMQ::Config.instance.segment_size) data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) with_channel do |ch| q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - q.publish_confirm data + q.publish_confirm msg_body end - sleep 0.1 with_channel do |ch| ch.prefetch 1 @@ -501,15 +499,14 @@ describe LavinMQ::StreamQueue do msgs.receive end - sleep 0.1 msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq 2 with_channel do |ch| q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - 4.times { q.publish_confirm data } + 2.times { q.publish_confirm msg_body } end - sleep 0.1 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil end From b4fc6aa8d6ceeae32ea9c18e30329983a269ffb2 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 4 Jun 2024 15:38:19 +0200 Subject: [PATCH 49/72] add option to use broker tracking when x-stream-offset is set by using x-stream-use-automatic-offset --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 7 ++++++- src/lavinmq/client/channel/stream_consumer.cr | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 379f465ed8..1e22915673 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -43,8 +43,13 @@ module LavinMQ::AMQP # Used once when a consumer is started # Populates `segment` and `position` by iterating through segments # until `offset` is found - def find_offset(offset, tag = nil) : Tuple(Int64, UInt32, UInt32) + def find_offset(offset, tag = nil, track_offset = false) : Tuple(Int64, UInt32, UInt32) raise ClosedError.new if @closed + if track_offset + consumer_last_offset = last_offset_by_consumer_tag(tag) + return find_offset_in_segments(consumer_last_offset) if consumer_last_offset + end + case offset when "first" then offset_at(@segments.first_key, 4u32) when "last" then offset_at(@segments.last_key, 4u32) diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index 0a44a2a9b3..55fbaf8644 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -15,7 +15,7 @@ module LavinMQ @tag = frame.consumer_tag validate_preconditions(frame) offset = frame.arguments["x-stream-offset"]? - @offset, @segment, @pos = stream_queue.find_offset(offset, @tag) + @offset, @segment, @pos = stream_queue.find_offset(offset, @tag, @track_offset) super end @@ -39,7 +39,7 @@ module LavinMQ when Nil @track_offset = true unless @tag.starts_with?("amq.ctag-") when Int, Time, "first", "next", "last" - @track_offset = false + @track_offset = true if frame.arguments["x-stream-use-automatic-offset"]? else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") end end From 7018f31893a642e5211f7de8a652e1cc24d975eb Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 4 Jun 2024 15:38:30 +0200 Subject: [PATCH 50/72] add spec --- spec/stream_queue_spec.cr | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 9e29d8f759..47efcada2b 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -452,6 +452,23 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 end + it "should use saved offset if x-stream-offset & x-stream-use-automatic-offset is set" do + queue_name = Random::Secure.hex + consumer_tag = Random::Secure.hex + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-use-automatic-offset": true}) + + StreamQueueSpecHelpers.publish(queue_name, 2) + + # get message without x-stream-offset, tracks offset + msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 + sleep 0.1 + + # consume with x-stream-offset set, should consume the same message again + msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 2 + end + it "cleanup_consumer_offsets removes outdated offset" do queue_name = Random::Secure.hex vhost = Server.vhosts["/"] From 5796d2b0a796f653e89ad27d5eee6b13b0fe621b Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 10:36:29 +0200 Subject: [PATCH 51/72] no need to truncate mfile, it's being deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 1e22915673..31f660d41b 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -189,7 +189,7 @@ module LavinMQ::AMQP @consumer_offsets = MFile.new("#{@consumer_offset_path}.tmp", @consumer_offset_capacity) yield # fill the new file with correct data in this block File.rename "#{@consumer_offset_path}.tmp", @consumer_offset_path - old_consumer_offsets.close + old_consumer_offsets.close(truncate_to_size: false) end def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? From 4246e911ca6ef3512e4a4c1b118dff70729a905f Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 10:38:29 +0200 Subject: [PATCH 52/72] x-stream-use-automatic-offset -> x-stream-automatic-offset-tracking --- spec/stream_queue_spec.cr | 4 ++-- src/lavinmq/client/channel/stream_consumer.cr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 47efcada2b..a88af8e961 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -452,10 +452,10 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 end - it "should use saved offset if x-stream-offset & x-stream-use-automatic-offset is set" do + it "should use saved offset if x-stream-offset & x-stream-automatic-offset-tracking is set" do queue_name = Random::Secure.hex consumer_tag = Random::Secure.hex - c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-use-automatic-offset": true}) + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-automatic-offset-tracking": true}) StreamQueueSpecHelpers.publish(queue_name, 2) diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index 55fbaf8644..57680887da 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -39,7 +39,7 @@ module LavinMQ when Nil @track_offset = true unless @tag.starts_with?("amq.ctag-") when Int, Time, "first", "next", "last" - @track_offset = true if frame.arguments["x-stream-use-automatic-offset"]? + @track_offset = true if frame.arguments["x-stream-automatic-offset-tracking"]? else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") end end From a85a6ac48424386b2f40919ab5144ee12604877f Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 10:51:24 +0200 Subject: [PATCH 53/72] implement rename in mfile --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- src/lavinmq/mfile.cr | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 31f660d41b..94c4643946 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -188,7 +188,7 @@ module LavinMQ::AMQP old_consumer_offsets = @consumer_offsets @consumer_offsets = MFile.new("#{@consumer_offset_path}.tmp", @consumer_offset_capacity) yield # fill the new file with correct data in this block - File.rename "#{@consumer_offset_path}.tmp", @consumer_offset_path + @consumer_offsets.rename(@consumer_offset_path) old_consumer_offsets.close(truncate_to_size: false) end diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 751e087aab..5b6518800a 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -291,4 +291,9 @@ class MFile < IO raise IO::Error.from_errno("pread") if cnt == -1 cnt end + + def rename(new_path : String) : Nil + File.rename @path, new_path + @path = new_path + end end From 7fc3f858fee5ff6b814755552995120b34bbf94e Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 14:33:53 +0200 Subject: [PATCH 54/72] expand consumer offsets file if full --- spec/stream_queue_spec.cr | 25 +++++++++++++++++++ .../amqp/queue/stream_queue_message_store.cr | 12 +++++++++ 2 files changed, 37 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index a88af8e961..fca2523e0d 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -550,5 +550,30 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(c_tag).should eq nil end + + it "expands consumer offset file when needed" do + queue_name = Random::Secure.hex + vhost = Server.vhosts["/"] + consumer_tag_prefix = "ctag-" + StreamQueueSpecHelpers.publish(queue_name, 1) + + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + 2000.times do |i| + next if i == 0 + msg_store.update_consumer_offset("#{consumer_tag_prefix}#{i}", i) + end + msg_store.close + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.@consumer_offsets.size.should eq 34_875 + + 2000.times do |i| + next if i == 0 + msg_store.last_offset_by_consumer_tag("#{consumer_tag_prefix}#{i}").should eq i + end + + msg_store.close + end end end diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 94c4643946..66f0004586 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -160,11 +160,23 @@ module LavinMQ::AMQP end def store_consumer_offset(consumer_tag : String, new_offset : Int64) + expand_consumer_offset_file if consumer_offset_file_full?(consumer_tag) @consumer_offsets.write_bytes AMQ::Protocol::ShortString.new(consumer_tag) @consumer_offset_positions[consumer_tag] = @consumer_offsets.size @consumer_offsets.write_bytes new_offset end + def consumer_offset_file_full?(consumer_tag) + (@consumer_offsets.size + consumer_tag.bytesize + 8) >= @consumer_offsets.capacity + end + + def expand_consumer_offset_file + pos = @consumer_offsets.size + @consumer_offset_capacity += 32_768 + @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) + @consumer_offsets.resize(pos) + end + def cleanup_consumer_offsets return if @consumer_offsets.size.zero? From 508762ce4bc0191b5701fd5ea201370e7da395df Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 14:34:13 +0200 Subject: [PATCH 55/72] remove unused code --- src/lavinmq/mfile.cr | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 5b6518800a..9d9c87456b 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -203,14 +203,6 @@ class MFile < IO @size = new_size end - def write_at(pos : Int64, slice : Bytes) : Nil - raise ClosedError.new if @closed - end_pos = pos + slice.size - raise IO::EOFError.new if end_pos > @capacity - @size = end_pos if end_pos > @size - slice.copy_to(buffer + pos, slice.size) - end - def read(slice : Bytes) pos = @pos len = Math.min(slice.size, @size - pos) From c1c0c5f13c8cadea912fa4cbd19a596757269b30 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 17 Jun 2024 14:36:20 +0200 Subject: [PATCH 56/72] use LittleEndian MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 66f0004586..8138ae782c 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -130,7 +130,7 @@ module LavinMQ::AMQP def last_offset_by_consumer_tag(consumer_tag) if pos = @consumer_offset_positions[consumer_tag]? tx = @consumer_offsets.to_slice(pos, 8) - return IO::ByteFormat::SystemEndian.decode(Int64, tx) + return IO::ByteFormat::LittleEndian.decode(Int64, tx) end end From 455fe9da231118203a809684ce176eb819f12c1f Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 19 Jun 2024 14:40:20 +0200 Subject: [PATCH 57/72] remove instance variables --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 8138ae782c..d15457746e 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -12,17 +12,14 @@ module LavinMQ::AMQP @segment_last_ts = Hash(UInt32, Int64).new(0i64) # used for max-age @offset_index = Hash(UInt32, Int64).new # segment_id => offset of first msg @timestamp_index = Hash(UInt32, Int64).new # segment_id => ts of first msg - @consumer_offset_path : String @consumer_offsets : MFile @consumer_offset_positions = Hash(String, Int64).new # used for consumer offsets - @consumer_offset_capacity = 32_768 def initialize(*args, **kwargs) super @last_offset = get_last_offset build_segment_indexes - @consumer_offset_path = File.join(@queue_data_dir, "consumer_offsets") - @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) + @consumer_offsets = MFile.new(File.join(@queue_data_dir, "consumer_offsets"), 32 * 1024) @consumer_offset_positions = restore_consumer_offset_positions drop_overflow end @@ -172,8 +169,7 @@ module LavinMQ::AMQP def expand_consumer_offset_file pos = @consumer_offsets.size - @consumer_offset_capacity += 32_768 - @consumer_offsets = MFile.new(@consumer_offset_path, @consumer_offset_capacity) + @consumer_offsets = MFile.new(@consumer_offsets.path, @consumer_offsets.capacity + 32 * 1024) @consumer_offsets.resize(pos) end @@ -198,9 +194,9 @@ module LavinMQ::AMQP def replace_offsets_file(&) old_consumer_offsets = @consumer_offsets - @consumer_offsets = MFile.new("#{@consumer_offset_path}.tmp", @consumer_offset_capacity) + @consumer_offsets = MFile.new("#{@consumer_offsets.path}.tmp", 32 * 1024) yield # fill the new file with correct data in this block - @consumer_offsets.rename(@consumer_offset_path) + @consumer_offsets.rename(@consumer_offsets.path.sub(".tmp","")) old_consumer_offsets.close(truncate_to_size: false) end From fcc0498d722f122fc8d0df9aa467a066b791e294 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 19 Jun 2024 14:42:41 +0200 Subject: [PATCH 58/72] use old_consumer_offsets.path --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index d15457746e..69f3a2ae6e 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -194,9 +194,9 @@ module LavinMQ::AMQP def replace_offsets_file(&) old_consumer_offsets = @consumer_offsets - @consumer_offsets = MFile.new("#{@consumer_offsets.path}.tmp", 32 * 1024) + @consumer_offsets = MFile.new("#{old_consumer_offsets.path}.tmp", 32 * 1024) yield # fill the new file with correct data in this block - @consumer_offsets.rename(@consumer_offsets.path.sub(".tmp","")) + @consumer_offsets.rename(old_consumer_offsets.path) old_consumer_offsets.close(truncate_to_size: false) end From cc8c1b75b54cac86bb7e6ad3feb2a7533ccfed1e Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 19 Jun 2024 15:17:18 +0200 Subject: [PATCH 59/72] start reading at pos=4 after IndexError in offset_at --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 69f3a2ae6e..7b5df1b99d 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -72,7 +72,7 @@ module LavinMQ::AMQP offset = offset_from_headers(msg.properties.headers) {offset, seg, pos} rescue ex : IndexError # first segment can be empty if message size >= segment size - return offset_at(seg + 1, pos, true) unless retried + return offset_at(seg + 1, 4_u32, true) unless retried raise ex end From e29a4c887ef62f1aebdcaf658abf74bf6554d4eb Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 19 Jun 2024 15:38:55 +0200 Subject: [PATCH 60/72] update specs to start amqp servers where needed --- spec/stream_queue_spec.cr | 282 ++++++++++++++++++++------------------ 1 file changed, 150 insertions(+), 132 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index fca2523e0d..5ed6630883 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -2,17 +2,17 @@ require "./spec_helper" require "./../src/lavinmq/amqp/queue" module StreamQueueSpecHelpers - def self.publish(queue_name, nr_of_messages) + def self.publish(s, queue_name, nr_of_messages) args = {"x-queue-type": "stream"} - with_channel do |ch| + with_channel(s) do |ch| q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) nr_of_messages.times { |i| q.publish "m#{i}" } end end - def self.consume_one(queue_name, c_tag, c_args = AMQP::Client::Arguments.new) + def self.consume_one(s, queue_name, c_tag, c_args = AMQP::Client::Arguments.new) args = {"x-queue-type": "stream"} - with_channel do |ch| + with_channel(s) do |ch| ch.prefetch 1 q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) msgs = Channel(AMQP::Client::DeliverMessage).new @@ -366,58 +366,63 @@ describe LavinMQ::StreamQueue do consumer_tag = Random::Secure.hex offset = 3 - StreamQueueSpecHelpers.publish(queue_name, offset + 1) - - offset.times { StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) } - sleep 0.1 + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, offset + 1) + offset.times { StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) } + sleep 0.1 - # consume again, should start from last offset automatically - msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq offset + 1 + # consume again, should start from last offset automatically + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq offset + 1 + end end it "reads offsets from file on init" do queue_name = Random::Secure.hex - vhost = Server.vhosts["/"] offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] tag_prefix = "ctag-" - StreamQueueSpecHelpers.publish(queue_name, 1) - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - offsets.each_with_index do |offset, i| - msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) - end - msg_store.close - sleep 0.1 + with_amqp_server do |s| + vhost = s.vhosts["/"] + StreamQueueSpecHelpers.publish(s, queue_name, 1) + + data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) + end + msg_store.close + sleep 0.1 - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - offsets.each_with_index do |offset, i| - msg_store.last_offset_by_consumer_tag(tag_prefix + i.to_s).should eq offset + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.last_offset_by_consumer_tag(tag_prefix + i.to_s).should eq offset + end + msg_store.close end - msg_store.close end it "only saves one entry per consumer tag" do queue_name = Random::Secure.hex - vhost = Server.vhosts["/"] offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] consumer_tag = "ctag-1" - StreamQueueSpecHelpers.publish(queue_name, 1) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - offsets.each do |offset| - msg_store.update_consumer_offset(consumer_tag, offset) - end - msg_store.close - sleep 0.1 + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each do |offset| + msg_store.update_consumer_offset(consumer_tag, offset) + end + msg_store.close + sleep 0.1 - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last - msg_store.@consumer_offsets.size.should eq 15 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last + msg_store.@consumer_offsets.size.should eq 15 - msg_store.close + msg_store.close + end end it "does not track offset if x-stream-offset is set" do @@ -425,14 +430,16 @@ describe LavinMQ::StreamQueue do consumer_tag = Random::Secure.hex c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0}) - StreamQueueSpecHelpers.publish(queue_name, 2) - msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 - - # should consume the same message again since tracking was not saved from last consume - msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 2) + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 + sleep 0.1 + + # should consume the same message again since tracking was not saved from last consume + msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 + end end it "should not use saved offset if x-stream-offset is set" do @@ -440,16 +447,18 @@ describe LavinMQ::StreamQueue do consumer_tag = Random::Secure.hex c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0}) - StreamQueueSpecHelpers.publish(queue_name, 2) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 2) - # get message without x-stream-offset, tracks offset - msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag) - StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 + # get message without x-stream-offset, tracks offset + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 + sleep 0.1 - # consume with x-stream-offset set, should consume the same message again - msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 + # consume with x-stream-offset set, should consume the same message again + msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 1 + end end it "should use saved offset if x-stream-offset & x-stream-automatic-offset-tracking is set" do @@ -457,123 +466,132 @@ describe LavinMQ::StreamQueue do consumer_tag = Random::Secure.hex c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-automatic-offset-tracking": true}) - StreamQueueSpecHelpers.publish(queue_name, 2) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 2) - # get message without x-stream-offset, tracks offset - msg = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 + # get message without x-stream-offset, tracks offset + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 + sleep 0.1 - # consume with x-stream-offset set, should consume the same message again - msg_2 = StreamQueueSpecHelpers.consume_one(queue_name, consumer_tag, c_args) - StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 2 + # consume with x-stream-offset set, should consume the same message again + msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 2 + end end it "cleanup_consumer_offsets removes outdated offset" do queue_name = Random::Secure.hex - vhost = Server.vhosts["/"] offsets = [84_i64, -10_i64] tag_prefix = "ctag-" - StreamQueueSpecHelpers.publish(queue_name, 1) - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - offsets.each_with_index do |offset, i| - msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) + + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each_with_index do |offset, i| + msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) + end + sleep 0.1 + msg_store.cleanup_consumer_offsets + msg_store.close + sleep 0.1 + + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil + msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] + msg_store.close end - sleep 0.1 - msg_store.cleanup_consumer_offsets - msg_store.close - sleep 0.1 - - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil - msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] - msg_store.close end it "runs cleanup when removing segment" do consumer_tag = "ctag-1" - vhost = Server.vhosts["/"] queue_name = Random::Secure.hex args = {"x-queue-type": "stream", "x-max-length": 1} msg_body = Bytes.new(LavinMQ::Config.instance.segment_size) - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - with_channel do |ch| - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - q.publish_confirm msg_body - end + with_amqp_server do |s| + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - with_channel do |ch| - ch.prefetch 1 - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - msgs = Channel(AMQP::Client::DeliverMessage).new - q.subscribe(no_ack: false, tag: consumer_tag) do |msg| - msgs.send msg - msg.ack + with_channel(s) do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + q.publish_confirm msg_body + end + + with_channel(s) do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + q.subscribe(no_ack: false, tag: consumer_tag) do |msg| + msgs.send msg + msg.ack + end + msgs.receive end - msgs.receive - end - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(consumer_tag).should eq 2 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq 2 - with_channel do |ch| - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - 2.times { q.publish_confirm msg_body } - end + with_channel(s) do |ch| + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + 2.times { q.publish_confirm msg_body } + end - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil + end end it "does not track offset if c-tag is auto-generated" do queue_name = Random::Secure.hex - StreamQueueSpecHelpers.publish(queue_name, 1) - args = {"x-queue-type": "stream"} - c_tag = "" - with_channel do |ch| - ch.prefetch 1 - q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) - msgs = Channel(AMQP::Client::DeliverMessage).new - c_tag = q.subscribe(no_ack: false) do |msg| - msgs.send msg - msg.ack + + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) + args = {"x-queue-type": "stream"} + c_tag = "" + with_channel(s) do |ch| + ch.prefetch 1 + q = ch.queue(queue_name, args: AMQP::Client::Arguments.new(args)) + msgs = Channel(AMQP::Client::DeliverMessage).new + c_tag = q.subscribe(no_ack: false) do |msg| + msgs.send msg + msg.ack + end + msgs.receive end - msgs.receive - end - sleep 0.1 - vhost = Server.vhosts["/"] - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.last_offset_by_consumer_tag(c_tag).should eq nil + sleep 0.1 + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.last_offset_by_consumer_tag(c_tag).should eq nil + end end it "expands consumer offset file when needed" do queue_name = Random::Secure.hex - vhost = Server.vhosts["/"] consumer_tag_prefix = "ctag-" - StreamQueueSpecHelpers.publish(queue_name, 1) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) - data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - 2000.times do |i| - next if i == 0 - msg_store.update_consumer_offset("#{consumer_tag_prefix}#{i}", i) - end - msg_store.close + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + 2000.times do |i| + next if i == 0 + msg_store.update_consumer_offset("#{consumer_tag_prefix}#{i}", i) + end + msg_store.close - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.@consumer_offsets.size.should eq 34_875 + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store.@consumer_offsets.size.should eq 34_875 - 2000.times do |i| - next if i == 0 - msg_store.last_offset_by_consumer_tag("#{consumer_tag_prefix}#{i}").should eq i - end + 2000.times do |i| + next if i == 0 + msg_store.last_offset_by_consumer_tag("#{consumer_tag_prefix}#{i}").should eq i + end - msg_store.close + msg_store.close + end end end end From c3a726630801b06300c5d65265093d695fa906be Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 19 Jun 2024 18:42:53 +0200 Subject: [PATCH 61/72] ameba:disable Metrics/CyclomaticComplexity for find_offset --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 7b5df1b99d..153bae7d0e 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -40,6 +40,7 @@ module LavinMQ::AMQP # Used once when a consumer is started # Populates `segment` and `position` by iterating through segments # until `offset` is found + # ameba:disable Metrics/CyclomaticComplexity def find_offset(offset, tag = nil, track_offset = false) : Tuple(Int64, UInt32, UInt32) raise ClosedError.new if @closed if track_offset From 9b91824555db0df72987172debccef5d8d5dd79d Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Fri, 21 Jun 2024 00:34:53 +0200 Subject: [PATCH 62/72] include tag size prefix byte in consumer_offset_file_full? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carl Hörberg --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 153bae7d0e..09ee28e972 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -165,7 +165,7 @@ module LavinMQ::AMQP end def consumer_offset_file_full?(consumer_tag) - (@consumer_offsets.size + consumer_tag.bytesize + 8) >= @consumer_offsets.capacity + (@consumer_offsets.size + 1 + consumer_tag.bytesize + 8) >= @consumer_offsets.capacity end def expand_consumer_offset_file From 17ac96af4c05ef646580389a8b55ee642da03d74 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Wed, 26 Jun 2024 13:06:48 +0200 Subject: [PATCH 63/72] only append consumer offsets file, compact when full, expand if still full. Use config.instance.segment_size --- spec/stream_queue_spec.cr | 52 ++++++++++++++++--- src/lavinmq/amqp/queue/stream_queue.cr | 4 +- .../amqp/queue/stream_queue_message_store.cr | 16 ++---- src/lavinmq/client/channel/stream_consumer.cr | 2 +- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 5ed6630883..4376cf0700 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -389,7 +389,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| - msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) + msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end msg_store.close sleep 0.1 @@ -402,7 +402,7 @@ describe LavinMQ::StreamQueue do end end - it "only saves one entry per consumer tag" do + it "appends consumer tag file" do queue_name = Random::Secure.hex offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] consumer_tag = "ctag-1" @@ -412,15 +412,53 @@ describe LavinMQ::StreamQueue do data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each do |offset| - msg_store.update_consumer_offset(consumer_tag, offset) + msg_store.store_consumer_offset(consumer_tag, offset) + end + bytesize = consumer_tag.bytesize + 1 + 8 + msg_store.@consumer_offsets.size.should eq bytesize*5 + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last + msg_store.close + end + end + + it "compacts consumer tag file on restart" do + queue_name = Random::Secure.hex + offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] + consumer_tag = "ctag-1" + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) + + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + offsets.each do |offset| + msg_store.store_consumer_offset(consumer_tag, offset) end msg_store.close - sleep 0.1 msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last - msg_store.@consumer_offsets.size.should eq 15 + bytesize = consumer_tag.bytesize + 1 + 8 + msg_store.@consumer_offsets.size.should eq bytesize + msg_store.close + end + end + it "compacts consumer tag file when full" do + queue_name = Random::Secure.hex + offsets = [84_i64, 24_i64, 1_i64, 100_i64, 42_i64] + consumer_tag = Random::Secure.hex(32) + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 1) + data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) + msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + bytesize = consumer_tag.bytesize + 1 + 8 + + offsets = (LavinMQ::Config.instance.segment_size / bytesize).to_i32 + 1 + offsets.times do |i| + msg_store.store_consumer_offset(consumer_tag, i) + end + msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets - 1 + msg_store.@consumer_offsets.size.should eq bytesize*2 msg_store.close end end @@ -491,7 +529,7 @@ describe LavinMQ::StreamQueue do data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| - msg_store.update_consumer_offset(tag_prefix + i.to_s, offset) + msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end sleep 0.1 msg_store.cleanup_consumer_offsets @@ -578,7 +616,7 @@ describe LavinMQ::StreamQueue do msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) 2000.times do |i| next if i == 0 - msg_store.update_consumer_offset("#{consumer_tag_prefix}#{i}", i) + msg_store.store_consumer_offset("#{consumer_tag_prefix}#{i}", i) end msg_store.close diff --git a/src/lavinmq/amqp/queue/stream_queue.cr b/src/lavinmq/amqp/queue/stream_queue.cr index ee11039c88..cd909c0d6c 100644 --- a/src/lavinmq/amqp/queue/stream_queue.cr +++ b/src/lavinmq/amqp/queue/stream_queue.cr @@ -79,8 +79,8 @@ module LavinMQ::AMQP end end - def update_consumer_offset(consumer_tag : String, offset : Int64) : Nil - stream_queue_msg_store.update_consumer_offset(consumer_tag, offset) + def store_consumer_offset(consumer_tag : String, offset : Int64) : Nil + stream_queue_msg_store.store_consumer_offset(consumer_tag, offset) end # yield the next message in the ready queue diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 09ee28e972..56341eb369 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -19,7 +19,7 @@ module LavinMQ::AMQP super @last_offset = get_last_offset build_segment_indexes - @consumer_offsets = MFile.new(File.join(@queue_data_dir, "consumer_offsets"), 32 * 1024) + @consumer_offsets = MFile.new(File.join(@queue_data_dir, "consumer_offsets"), Config.instance.segment_size) @consumer_offset_positions = restore_consumer_offset_positions drop_overflow end @@ -149,19 +149,13 @@ module LavinMQ::AMQP positions end - def update_consumer_offset(consumer_tag : String, new_offset : Int64) - if pos = @consumer_offset_positions[consumer_tag]? - IO::ByteFormat::SystemEndian.encode(new_offset, @consumer_offsets.to_slice(pos, 8, false)) - else - store_consumer_offset(consumer_tag, new_offset) - end - end - def store_consumer_offset(consumer_tag : String, new_offset : Int64) + cleanup_consumer_offsets if consumer_offset_file_full?(consumer_tag) expand_consumer_offset_file if consumer_offset_file_full?(consumer_tag) @consumer_offsets.write_bytes AMQ::Protocol::ShortString.new(consumer_tag) @consumer_offset_positions[consumer_tag] = @consumer_offsets.size @consumer_offsets.write_bytes new_offset + # replicate end def consumer_offset_file_full?(consumer_tag) @@ -170,7 +164,7 @@ module LavinMQ::AMQP def expand_consumer_offset_file pos = @consumer_offsets.size - @consumer_offsets = MFile.new(@consumer_offsets.path, @consumer_offsets.capacity + 32 * 1024) + @consumer_offsets = MFile.new(@consumer_offsets.path, @consumer_offsets.capacity + Config.instance.segment_size) @consumer_offsets.resize(pos) end @@ -195,7 +189,7 @@ module LavinMQ::AMQP def replace_offsets_file(&) old_consumer_offsets = @consumer_offsets - @consumer_offsets = MFile.new("#{old_consumer_offsets.path}.tmp", 32 * 1024) + @consumer_offsets = MFile.new("#{old_consumer_offsets.path}.tmp", Config.instance.segment_size) yield # fill the new file with correct data in this block @consumer_offsets.rename(old_consumer_offsets.path) old_consumer_offsets.close(truncate_to_size: false) diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr index 57680887da..2dfc4a5edd 100644 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ b/src/lavinmq/client/channel/stream_consumer.cr @@ -88,7 +88,7 @@ module LavinMQ end def ack(sp) - stream_queue.update_consumer_offset(@tag, @offset) if @track_offset + stream_queue.store_consumer_offset(@tag, @offset) if @track_offset super end From f22d5cbec7b83670eac6c381e0b0f46d79dbc584 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 27 Jun 2024 14:41:47 +0200 Subject: [PATCH 64/72] set capacity of consumer offsets file to 1000*current size when compacting --- spec/stream_queue_spec.cr | 27 +++++++++---------- .../amqp/queue/stream_queue_message_store.cr | 10 +++---- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 4376cf0700..7fce7824e1 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -608,27 +608,24 @@ describe LavinMQ::StreamQueue do it "expands consumer offset file when needed" do queue_name = Random::Secure.hex - consumer_tag_prefix = "ctag-" + consumer_tag_prefix = Random::Secure.hex(32) with_amqp_server do |s| StreamQueueSpecHelpers.publish(s, queue_name, 1) - data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - 2000.times do |i| - next if i == 0 - msg_store.store_consumer_offset("#{consumer_tag_prefix}#{i}", i) + one_offset_bytesize = "#{consumer_tag_prefix}#{1000}".bytesize + 1 + 8 + offsets = (LavinMQ::Config.instance.segment_size / one_offset_bytesize).to_i32 + 1 + bytesize = 0 + offsets.times do |i| + consumer_tag = "#{consumer_tag_prefix}#{i + 1000}" + msg_store.store_consumer_offset(consumer_tag, i + 1000) + bytesize += consumer_tag.bytesize + 1 + 8 end - msg_store.close - - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - msg_store.@consumer_offsets.size.should eq 34_875 - - 2000.times do |i| - next if i == 0 - msg_store.last_offset_by_consumer_tag("#{consumer_tag_prefix}#{i}").should eq i + msg_store.@consumer_offsets.size.should eq bytesize + msg_store.@consumer_offsets.size.should be > LavinMQ::Config.instance.segment_size + offsets.times do |i| + msg_store.last_offset_by_consumer_tag("#{consumer_tag_prefix}#{i + 1000}").should eq i + 1000 end - - msg_store.close end end end diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 56341eb369..6e56f46c84 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -151,7 +151,6 @@ module LavinMQ::AMQP def store_consumer_offset(consumer_tag : String, new_offset : Int64) cleanup_consumer_offsets if consumer_offset_file_full?(consumer_tag) - expand_consumer_offset_file if consumer_offset_file_full?(consumer_tag) @consumer_offsets.write_bytes AMQ::Protocol::ShortString.new(consumer_tag) @consumer_offset_positions[consumer_tag] = @consumer_offsets.size @consumer_offsets.write_bytes new_offset @@ -173,23 +172,24 @@ module LavinMQ::AMQP offsets_to_save = Hash(String, Int64).new lowest_offset_in_stream, _seg, _pos = offset_at(@segments.first_key, 4u32) + capacity = 0 @consumer_offset_positions.each do |ctag, _pos| if offset = last_offset_by_consumer_tag(ctag) offsets_to_save[ctag] = offset if offset >= lowest_offset_in_stream + capacity += ctag.bytesize + 1 + 8 end end - @consumer_offset_positions = Hash(String, Int64).new - replace_offsets_file do + replace_offsets_file(capacity * 1000) do offsets_to_save.each do |ctag, offset| store_consumer_offset(ctag, offset) end end end - def replace_offsets_file(&) + def replace_offsets_file(capacity : Int, &) old_consumer_offsets = @consumer_offsets - @consumer_offsets = MFile.new("#{old_consumer_offsets.path}.tmp", Config.instance.segment_size) + @consumer_offsets = MFile.new("#{old_consumer_offsets.path}.tmp", capacity) yield # fill the new file with correct data in this block @consumer_offsets.rename(old_consumer_offsets.path) old_consumer_offsets.close(truncate_to_size: false) From 04c76502cd6f9bb135b1dabac7a4e7cb2763b111 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Thu, 27 Jun 2024 15:48:11 +0200 Subject: [PATCH 65/72] replicate consumer offsets file. remove unused code --- src/lavinmq/amqp/queue/stream_queue_message_store.cr | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 6e56f46c84..63f47f74ca 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -20,6 +20,7 @@ module LavinMQ::AMQP @last_offset = get_last_offset build_segment_indexes @consumer_offsets = MFile.new(File.join(@queue_data_dir, "consumer_offsets"), Config.instance.segment_size) + @replicator.try &.register_file @consumer_offsets @consumer_offset_positions = restore_consumer_offset_positions drop_overflow end @@ -154,19 +155,13 @@ module LavinMQ::AMQP @consumer_offsets.write_bytes AMQ::Protocol::ShortString.new(consumer_tag) @consumer_offset_positions[consumer_tag] = @consumer_offsets.size @consumer_offsets.write_bytes new_offset - # replicate + @replicator.try &.append(@consumer_offsets.path, (@consumer_offsets.size - consumer_tag.bytesize - 1 - 8).to_i32) end def consumer_offset_file_full?(consumer_tag) (@consumer_offsets.size + 1 + consumer_tag.bytesize + 8) >= @consumer_offsets.capacity end - def expand_consumer_offset_file - pos = @consumer_offsets.size - @consumer_offsets = MFile.new(@consumer_offsets.path, @consumer_offsets.capacity + Config.instance.segment_size) - @consumer_offsets.resize(pos) - end - def cleanup_consumer_offsets return if @consumer_offsets.size.zero? @@ -193,6 +188,7 @@ module LavinMQ::AMQP yield # fill the new file with correct data in this block @consumer_offsets.rename(old_consumer_offsets.path) old_consumer_offsets.close(truncate_to_size: false) + @replicator.try &.replace_file @consumer_offsets.path end def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? From 5222c9ed8dc82d233d686589bdd3224366c23ad5 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 12 Nov 2024 17:14:28 +0100 Subject: [PATCH 66/72] fixes to match changes from main --- spec/stream_queue_spec.cr | 26 ++--- .../amqp/queue/stream_queue_message_store.cr | 2 +- src/lavinmq/amqp/stream_consumer.cr | 14 ++- src/lavinmq/client/channel/stream_consumer.cr | 105 ------------------ 4 files changed, 26 insertions(+), 121 deletions(-) delete mode 100644 src/lavinmq/client/channel/stream_consumer.cr diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 7fce7824e1..e7d09c839d 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -33,7 +33,7 @@ module StreamQueueSpecHelpers end end -describe LavinMQ::StreamQueue do +describe LavinMQ::AMQP::StreamQueue do stream_queue_args = LavinMQ::AMQP::Table.new({"x-queue-type": "stream"}) describe "Consume" do @@ -387,14 +387,14 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(vhost.data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end msg_store.close sleep 0.1 - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| msg_store.last_offset_by_consumer_tag(tag_prefix + i.to_s).should eq offset end @@ -410,7 +410,7 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each do |offset| msg_store.store_consumer_offset(consumer_tag, offset) end @@ -429,13 +429,13 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each do |offset| msg_store.store_consumer_offset(consumer_tag, offset) end msg_store.close - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq offsets.last bytesize = consumer_tag.bytesize + 1 + 8 msg_store.@consumer_offsets.size.should eq bytesize @@ -450,7 +450,7 @@ describe LavinMQ::StreamQueue do with_amqp_server do |s| StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) bytesize = consumer_tag.bytesize + 1 + 8 offsets = (LavinMQ::Config.instance.segment_size / bytesize).to_i32 + 1 @@ -527,7 +527,7 @@ describe LavinMQ::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end @@ -536,7 +536,7 @@ describe LavinMQ::StreamQueue do msg_store.close sleep 0.1 - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil msg_store.last_offset_by_consumer_tag(tag_prefix + 0.to_s).should eq offsets[0] msg_store.close @@ -568,7 +568,7 @@ describe LavinMQ::StreamQueue do msgs.receive end - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq 2 with_channel(s) do |ch| @@ -576,7 +576,7 @@ describe LavinMQ::StreamQueue do 2.times { q.publish_confirm msg_body } end - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(consumer_tag).should eq nil end end @@ -601,7 +601,7 @@ describe LavinMQ::StreamQueue do sleep 0.1 data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(c_tag).should eq nil end end @@ -612,7 +612,7 @@ describe LavinMQ::StreamQueue do with_amqp_server do |s| StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) - msg_store = LavinMQ::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) + msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) one_offset_bytesize = "#{consumer_tag_prefix}#{1000}".bytesize + 1 + 8 offsets = (LavinMQ::Config.instance.segment_size / one_offset_bytesize).to_i32 + 1 bytesize = 0 diff --git a/src/lavinmq/amqp/queue/stream_queue_message_store.cr b/src/lavinmq/amqp/queue/stream_queue_message_store.cr index 63f47f74ca..42ea4268c9 100644 --- a/src/lavinmq/amqp/queue/stream_queue_message_store.cr +++ b/src/lavinmq/amqp/queue/stream_queue_message_store.cr @@ -191,7 +191,7 @@ module LavinMQ::AMQP @replicator.try &.replace_file @consumer_offsets.path end - def shift?(consumer : Client::Channel::StreamConsumer) : Envelope? + def shift?(consumer : AMQP::StreamConsumer) : Envelope? raise ClosedError.new if @closed if env = shift_requeued(consumer.requeued) diff --git a/src/lavinmq/amqp/stream_consumer.cr b/src/lavinmq/amqp/stream_consumer.cr index 7c92b6580e..52204fe82c 100644 --- a/src/lavinmq/amqp/stream_consumer.cr +++ b/src/lavinmq/amqp/stream_consumer.cr @@ -11,11 +11,13 @@ module LavinMQ getter requeued = Deque(SegmentPosition).new @filter = Array(String).new @match_unfiltered = false + @track_offset = false def initialize(@channel : Client::Channel, @queue : StreamQueue, frame : AMQP::Frame::Basic::Consume) + @tag = frame.consumer_tag validate_preconditions(frame) offset = frame.arguments["x-stream-offset"]? - @offset, @segment, @pos = stream_queue.find_offset(offset) + @offset, @segment, @pos = stream_queue.find_offset(offset, @tag, @track_offset) super end @@ -36,7 +38,10 @@ module LavinMQ raise LavinMQ::Error::PreconditionFailed.new("x-priority not supported on stream queues") end case frame.arguments["x-stream-offset"]? - when Nil, Int, Time, "first", "next", "last" + when Nil + @track_offset = true unless @tag.starts_with?("amq.ctag-") + when Int, Time, "first", "next", "last" + @track_offset = true if frame.arguments["x-stream-automatic-offset-tracking"]? else raise LavinMQ::Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") end case filter = frame.arguments["x-stream-filter"]? @@ -98,6 +103,11 @@ module LavinMQ @queue.as(StreamQueue) end + def ack(sp) + stream_queue.store_consumer_offset(@tag, @offset) if @track_offset + super + end + def reject(sp, requeue : Bool) super if requeue diff --git a/src/lavinmq/client/channel/stream_consumer.cr b/src/lavinmq/client/channel/stream_consumer.cr deleted file mode 100644 index 2dfc4a5edd..0000000000 --- a/src/lavinmq/client/channel/stream_consumer.cr +++ /dev/null @@ -1,105 +0,0 @@ -require "./consumer" -require "../../segment_position" - -module LavinMQ - class Client - class Channel - class StreamConsumer < LavinMQ::Client::Channel::Consumer - property offset : Int64 - property segment : UInt32 - property pos : UInt32 - getter requeued = Deque(SegmentPosition).new - @track_offset = false - - def initialize(@channel : Client::Channel, @queue : StreamQueue, frame : AMQP::Frame::Basic::Consume) - @tag = frame.consumer_tag - validate_preconditions(frame) - offset = frame.arguments["x-stream-offset"]? - @offset, @segment, @pos = stream_queue.find_offset(offset, @tag, @track_offset) - super - end - - private def validate_preconditions(frame) - if frame.exclusive - raise Error::PreconditionFailed.new("Stream consumers must not be exclusive") - end - if frame.no_ack - raise Error::PreconditionFailed.new("Stream consumers must acknowledge messages") - end - if @channel.prefetch_count.zero? - raise Error::PreconditionFailed.new("Stream consumers must have a prefetch limit") - end - unless @channel.global_prefetch_count.zero? - raise Error::PreconditionFailed.new("Stream consumers does not support global prefetch limit") - end - if frame.arguments.has_key? "x-priority" - raise Error::PreconditionFailed.new("x-priority not supported on stream queues") - end - case frame.arguments["x-stream-offset"]? - when Nil - @track_offset = true unless @tag.starts_with?("amq.ctag-") - when Int, Time, "first", "next", "last" - @track_offset = true if frame.arguments["x-stream-automatic-offset-tracking"]? - else raise Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") - end - end - - private def deliver_loop - i = 0 - loop do - wait_for_capacity - loop do - raise ClosedError.new if @closed - next if wait_for_queue_ready - next if wait_for_paused_queue - next if wait_for_flow - break - end - {% unless flag?(:release) %} - @log.debug { "Getting a new message" } - {% end %} - stream_queue.consume_get(self) do |env| - deliver(env.message, env.segment_position, env.redelivered) - end - Fiber.yield if (i &+= 1) % 32768 == 0 - end - rescue ex : ClosedError | Queue::ClosedError | Client::Channel::ClosedError | ::Channel::ClosedError - @log.debug { "deliver loop exiting: #{ex.inspect}" } - end - - private def wait_for_queue_ready - if @offset > stream_queue.last_offset && @requeued.empty? - @log.debug { "Waiting for queue not to be empty" } - select - when stream_queue.new_messages.receive - @log.debug { "Queue is not empty" } - when @has_requeued.receive - @log.debug { "Got a requeued message" } - when @notify_closed.receive - end - return true - end - end - - @has_requeued = ::Channel(Nil).new - - private def stream_queue : StreamQueue - @queue.as(StreamQueue) - end - - def ack(sp) - stream_queue.store_consumer_offset(@tag, @offset) if @track_offset - super - end - - def reject(sp, requeue : Bool) - super - if requeue - @requeued.push(sp) - @has_requeued.try_send? nil if @requeued.size == 1 - end - end - end - end - end -end From 397222e3ae7140c238d0216486611ec7787f13f7 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Tue, 12 Nov 2024 17:16:50 +0100 Subject: [PATCH 67/72] lint --- spec/stream_queue_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index e7d09c839d..d7f048cb9c 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -613,7 +613,7 @@ describe LavinMQ::AMQP::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 1) data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) - one_offset_bytesize = "#{consumer_tag_prefix}#{1000}".bytesize + 1 + 8 + one_offset_bytesize = "#{consumer_tag_prefix}1000".bytesize + 1 + 8 offsets = (LavinMQ::Config.instance.segment_size / one_offset_bytesize).to_i32 + 1 bytesize = 0 offsets.times do |i| From 18055c7cb42269978487d04a6e74ea3cfa038dcd Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Fri, 15 Nov 2024 11:38:00 +0100 Subject: [PATCH 68/72] add .seconds to timespans --- spec/stream_queue_spec.cr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index d7f048cb9c..361d8b8d96 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -369,7 +369,7 @@ describe LavinMQ::AMQP::StreamQueue do with_amqp_server do |s| StreamQueueSpecHelpers.publish(s, queue_name, offset + 1) offset.times { StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) } - sleep 0.1 + sleep 0.1.seconds # consume again, should start from last offset automatically msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) @@ -392,7 +392,7 @@ describe LavinMQ::AMQP::StreamQueue do msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end msg_store.close - sleep 0.1 + wait_for { msg_store.@closed } msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) offsets.each_with_index do |offset, i| @@ -472,7 +472,7 @@ describe LavinMQ::AMQP::StreamQueue do StreamQueueSpecHelpers.publish(s, queue_name, 2) msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 + sleep 0.1.seconds # should consume the same message again since tracking was not saved from last consume msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) @@ -491,7 +491,7 @@ describe LavinMQ::AMQP::StreamQueue do # get message without x-stream-offset, tracks offset msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag) StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 + sleep 0.1.seconds # consume with x-stream-offset set, should consume the same message again msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) @@ -510,7 +510,7 @@ describe LavinMQ::AMQP::StreamQueue do # get message without x-stream-offset, tracks offset msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 - sleep 0.1 + sleep 0.1.seconds # consume with x-stream-offset set, should consume the same message again msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) @@ -531,10 +531,10 @@ describe LavinMQ::AMQP::StreamQueue do offsets.each_with_index do |offset, i| msg_store.store_consumer_offset(tag_prefix + i.to_s, offset) end - sleep 0.1 + sleep 0.1.seconds msg_store.cleanup_consumer_offsets msg_store.close - sleep 0.1 + sleep 0.1.seconds msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(tag_prefix + 1.to_s).should eq nil @@ -599,7 +599,7 @@ describe LavinMQ::AMQP::StreamQueue do msgs.receive end - sleep 0.1 + sleep 0.1.seconds data_dir = File.join(s.vhosts["/"].data_dir, Digest::SHA1.hexdigest queue_name) msg_store = LavinMQ::AMQP::StreamQueue::StreamQueueMessageStore.new(data_dir, nil) msg_store.last_offset_by_consumer_tag(c_tag).should eq nil From d3d69ab9d06bcf1fa923c2508c32e3fc4ebda189 Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 27 Jan 2025 14:59:34 +0100 Subject: [PATCH 69/72] read_only is no longer used --- src/lavinmq/mfile.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lavinmq/mfile.cr b/src/lavinmq/mfile.cr index 9d9c87456b..2a1c2402d4 100644 --- a/src/lavinmq/mfile.cr +++ b/src/lavinmq/mfile.cr @@ -248,12 +248,12 @@ class MFile < IO end def to_slice - Bytes.new(buffer, @size, read_only: true) + Bytes.new(buffer, @size) end - def to_slice(pos, size, read_only = true) + def to_slice(pos, size) raise IO::EOFError.new if pos + size > @size - Bytes.new(buffer + pos, size, read_only: read_only) + Bytes.new(buffer + pos, size) end def advise(advice : Advice, addr = buffer, offset = 0, length = @capacity) : Nil From 7e94d7092336ecfae810a7e645407d9211acaa8b Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 27 Jan 2025 15:03:03 +0100 Subject: [PATCH 70/72] fix specs --- spec/stream_queue_spec.cr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index 361d8b8d96..efa3c32320 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -360,6 +360,8 @@ describe LavinMQ::AMQP::StreamQueue do msg.body_io.to_s.should eq "msg with filter 2" end end + end + end describe "Automatic consumer offset tracking" do it "resumes from last offset on reconnect" do queue_name = Random::Secure.hex From 9666a4ff6260a88edc73d34ae2857cf9edfa51fc Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 27 Jan 2025 15:52:23 +0100 Subject: [PATCH 71/72] handle false for x-stream-automatic-offset-tracking and add spec for it. Lint: Refactor validate_preconditions, move validate_stream_offset and validate_stream_filter to separate methods --- spec/stream_queue_spec.cr | 29 +++++++++++++++++++---- src/lavinmq/amqp/stream_consumer.cr | 36 +++++++++++++++++++++-------- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/spec/stream_queue_spec.cr b/spec/stream_queue_spec.cr index efa3c32320..ed2bafd7fd 100644 --- a/spec/stream_queue_spec.cr +++ b/spec/stream_queue_spec.cr @@ -504,19 +504,38 @@ describe LavinMQ::AMQP::StreamQueue do it "should use saved offset if x-stream-offset & x-stream-automatic-offset-tracking is set" do queue_name = Random::Secure.hex consumer_tag = Random::Secure.hex - c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-automatic-offset-tracking": true}) + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-automatic-offset-tracking": "true"}) with_amqp_server do |s| StreamQueueSpecHelpers.publish(s, queue_name, 2) - # get message without x-stream-offset, tracks offset + # tracks offset msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 sleep 0.1.seconds - # consume with x-stream-offset set, should consume the same message again - msg_2 = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) - StreamQueueSpecHelpers.offset_from_headers(msg_2.properties.headers).should eq 2 + # should continue from tracked offset + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 2 + end + end + + it "should not use saved offset if x-stream-automatic-offset-tracking is false" do + queue_name = Random::Secure.hex + consumer_tag = Random::Secure.hex + c_args = AMQP::Client::Arguments.new({"x-stream-offset": 0, "x-stream-automatic-offset-tracking": "false"}) + + with_amqp_server do |s| + StreamQueueSpecHelpers.publish(s, queue_name, 2) + + # does not track offset + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 + sleep 0.1.seconds + + # should consume the same message again, no tracked offset + msg = StreamQueueSpecHelpers.consume_one(s, queue_name, consumer_tag, c_args) + StreamQueueSpecHelpers.offset_from_headers(msg.properties.headers).should eq 1 end end diff --git a/src/lavinmq/amqp/stream_consumer.cr b/src/lavinmq/amqp/stream_consumer.cr index 52204fe82c..cb323ed1d7 100644 --- a/src/lavinmq/amqp/stream_consumer.cr +++ b/src/lavinmq/amqp/stream_consumer.cr @@ -37,27 +37,43 @@ module LavinMQ if frame.arguments.has_key? "x-priority" raise LavinMQ::Error::PreconditionFailed.new("x-priority not supported on stream queues") end + validate_stream_offset(frame) + validate_stream_filter(frame.arguments["x-stream-filter"]?) + case match_unfiltered = frame.arguments["x-stream-match-unfiltered"]? + when Bool + @match_unfiltered = match_unfiltered + when Nil + # noop + else raise LavinMQ::Error::PreconditionFailed.new("x-stream-match-unfiltered must be a boolean") + end + end + + private def validate_stream_offset(frame) case frame.arguments["x-stream-offset"]? when Nil @track_offset = true unless @tag.starts_with?("amq.ctag-") when Int, Time, "first", "next", "last" - @track_offset = true if frame.arguments["x-stream-automatic-offset-tracking"]? + case frame.arguments["x-stream-automatic-offset-tracking"]? + when Bool + @track_offset = frame.arguments["x-stream-automatic-offset-tracking"]?.as(Bool) + when String + @track_offset = frame.arguments["x-stream-automatic-offset-tracking"]? == "true" + end else raise LavinMQ::Error::PreconditionFailed.new("x-stream-offset must be an integer, a timestamp, 'first', 'next' or 'last'") end - case filter = frame.arguments["x-stream-filter"]? + end + + private def validate_stream_filter(arg) + case arg when String - @filter = filter.split(',').sort! + @filter = arg.split(',').sort! when Nil # noop else raise LavinMQ::Error::PreconditionFailed.new("x-stream-filter-value must be a string") end - case match_unfiltered = frame.arguments["x-stream-match-unfiltered"]? - when Bool - @match_unfiltered = match_unfiltered - when Nil - # noop - else raise LavinMQ::Error::PreconditionFailed.new("x-stream-match-unfiltered must be a boolean") - end + end + + private def validate_offset_tracking(arg) end private def deliver_loop From e6fd1eeedb32946d98789590f27ed190bdc5c21a Mon Sep 17 00:00:00 2001 From: Viktor Erlingsson Date: Mon, 27 Jan 2025 15:55:45 +0100 Subject: [PATCH 72/72] fixup! handle false for x-stream-automatic-offset-tracking and add spec for it. Lint: Refactor validate_preconditions, move validate_stream_offset and validate_stream_filter to separate methods --- src/lavinmq/amqp/stream_consumer.cr | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/lavinmq/amqp/stream_consumer.cr b/src/lavinmq/amqp/stream_consumer.cr index cb323ed1d7..32a953cb67 100644 --- a/src/lavinmq/amqp/stream_consumer.cr +++ b/src/lavinmq/amqp/stream_consumer.cr @@ -73,9 +73,6 @@ module LavinMQ end end - private def validate_offset_tracking(arg) - end - private def deliver_loop i = 0 loop do