Skip to content

ARTEMIS-5573 and ARTEMIS-5975 Improve AMQP Size estimation#6323

Open
clebertsuconic wants to merge 2 commits intoapache:mainfrom
clebertsuconic:message-size-amqp
Open

ARTEMIS-5573 and ARTEMIS-5975 Improve AMQP Size estimation#6323
clebertsuconic wants to merge 2 commits intoapache:mainfrom
clebertsuconic:message-size-amqp

Conversation

@clebertsuconic
Copy link
Copy Markdown
Contributor

No description provided.

@clebertsuconic clebertsuconic force-pushed the message-size-amqp branch 8 times, most recently from f70cda5 to be90ac4 Compare March 29, 2026 21:44
@clebertsuconic clebertsuconic changed the title ARTEMIS-TBD Improving memory estimates on AMQP ARTEMIS-5573 and ARTEMIS-5975 Improve AMQP Size estimation and make it static Mar 29, 2026
@clebertsuconic clebertsuconic force-pushed the message-size-amqp branch 9 times, most recently from 0611d5a to 03b7176 Compare March 30, 2026 22:51
@clebertsuconic clebertsuconic marked this pull request as ready for review March 30, 2026 22:51
@clebertsuconic clebertsuconic force-pushed the message-size-amqp branch 8 times, most recently from 94efe35 to 70beb3a Compare April 1, 2026 13:07
@clebertsuconic clebertsuconic changed the title ARTEMIS-5573 and ARTEMIS-5975 Improve AMQP Size estimation and make it static ARTEMIS-5573 and ARTEMIS-5975 Improve AMQP Size estimation Apr 1, 2026
@tabish121 tabish121 self-requested a review April 1, 2026 14:14
Copy link
Copy Markdown
Contributor

@tabish121 tabish121 left a comment

Choose a reason for hiding this comment

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

Overall looks good, nice simplification of some of the message handling

@clebertsuconic clebertsuconic marked this pull request as draft April 7, 2026 21:51
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

I added a commit to be squashed, returning the RELOAD state and persisting the memory estimate on the storage (journal or paging).

I honestly don't think the scan will make any difference.. The real issue is that the estimates were wrong in the first place...

But this commit should settle any discussion.

I think There are already version tests in the Version tests. but i will make sure about it soon.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

for some reason I'm having to use a different persister, and I would have to digg on previous versions to understand why.

I will do some investigation tomorrow, but most likely I will need the V4 persister.

@clebertsuconic clebertsuconic force-pushed the message-size-amqp branch 7 times, most recently from 09bb86b to 540ad20 Compare April 8, 2026 00:58
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

The reason I had to add a V4 is because of Paging:

On this part here, I encode the number of queues and the queueIDs:

int queueIDsSize = buffer.readInt();
queueIDs = new long[queueIDsSize];
for (int i = 0; i < queueIDsSize; i++) {
queueIDs[i] = buffer.readLong();
}

At the point of the encoder I don't have any reference to the number of bytes used by its own decoder.

I'm adding one on V4 now, if in the future anything else is added, we will stop reading at that marker.

@clebertsuconic clebertsuconic force-pushed the message-size-amqp branch 4 times, most recently from 1cb4403 to fa1128e Compare April 8, 2026 01:26
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

I intend to squash the second commit on the first as soon some review is done on the new persister.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

I'm setting it as ready to review. but this commit should be squashed before merged.

@clebertsuconic
Copy link
Copy Markdown
Contributor Author

I needed to add versioning to AMQPLargeMessagePersister as well.

I could choose to skip memory calculations on large message... however I will prefer to keep it the same way as StandardMessage.

this is also better for future additions

- making it immutable to avoid races after updates like we had in the past
- avoiding scanning after reloading by persisting extra data on storage
@clebertsuconic
Copy link
Copy Markdown
Contributor Author

I added a commit addressing the comments, let me know when I can squash it please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants