Skip to content

Fix permanent event batching for infinite buffers - #325

Merged
josevalim merged 3 commits into
elixir-lang:mainfrom
pckrishnadas88:fix-infinite-buffer-permanents
Aug 5, 2026
Merged

Fix permanent event batching for infinite buffers#325
josevalim merged 3 commits into
elixir-lang:mainfrom
pckrishnadas88:fix-infinite-buffer-permanents

Conversation

@pckrishnadas88

Copy link
Copy Markdown
Contributor

Fixes the different behavior of Buffer.take_count_or_until_permanent/2
for infinite buffers.

Infinite buffers previously returned only the first permanent event.
This change collects consecutive permanent events, matching the behavior
of finite buffers.

Closes #317

Comment thread lib/gen_stage/buffer.ex
case :queue.peek(queue) do
{:value, {^infos, perm}} ->
{{:value, _}, queue} = :queue.out(queue)
take_permanents(queue, buffer - 1, infos, [perm | perms])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to worry about buffer going eventually negative? Can we add a test or is it bound to the queue size anyway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it is bounded by the queue size, since each permanent stored in the infinite buffer increments the count and the helper only decrements it when another permanent entry is actually present in the queue. I also added an assertion that the buffer count reaches 0 when all entries are consumed.

Comment thread lib/gen_stage/buffer.ex
{queue, buffer, perms} =
take_permanents(queue, buffer - 1, infos, [perm])

{:ok, {queue, buffer, infos}, counter, :lists.reverse(temps), :lists.reverse(perms)}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about the other clauses? Is this an issue:

When the last requested temporary event makes counter - 1 == 0, recursion enters the zero-counter clause before examining the following permanent entries and take_permanents/4 is never called:

buffer = Buffer.new(:infinity)
{buffer, _, _} = Buffer.store_temporary(buffer, [:temp], :first)
{:ok, buffer} = Buffer.store_permanent_unless_empty(buffer, :perm1)
{:ok, buffer} = Buffer.store_permanent_unless_empty(buffer, :perm2)

Buffer.take_count_or_until_permanent(buffer, 1)
#=> returns temps: [:temp], perms: [] and both permanents remain buffered

If so, we need to add a test for it.

@pckrishnadas88 pckrishnadas88 Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was an issue. I added a regression test for this case and updated the infinite-buffer path to handle permanents when the counter reaches zero.

@josevalim
josevalim merged commit 73957a6 into elixir-lang:main Aug 5, 2026
1 of 2 checks passed
@josevalim

Copy link
Copy Markdown
Member

💚 💙 💜 💛 ❤️

@pckrishnadas88
pckrishnadas88 deleted the fix-infinite-buffer-permanents branch August 6, 2026 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Different behavior with Buffer.take_count_or_until_permanent/2 when Buffer is size :infinity

2 participants