ARTEMIS-4809 Allow configuring initial queue buffer size

[joshb1050]

In some setups, there could be a few hundred thousand queues that are created due to many consumers that are connecting. However, most of these are empty and stay empty for the entire day since there aren't necessarily messages to be sent. The 8K intermediateMessageReferences instantiates an 64KB buffer (Object[]). This means we have large allocation and live heap that ultimately remains empty for almost the entire day.

In this commit, we introduce initial-queue-buffer-size, which defaults to the current value of 8192. It can be set programmatically via
QueueConfiguration#setInitialQueueBufferSize(int).

Note that this must be a power of 2.

[jbertram]

Thanks for the PR!

I think this would be better as an address-setting so you could easily configure large swaths of queues. Right now there's no way to configure this via broker.xml. It can only be configured if you're directly creating the queue(s) programmatically.

Lastly, there's no tests or documentation for this new setting. Tests are mandatory to ensure the functionality works properly and to mitigate future regressions.

[joshb1050]

Thanks @jbertram! I have updated my PR to make this configurable on a per-address basis and via XML as well. I have also added tests and documentation for the functionality. Please let me know if there is a different approach I should be taking here.

[joshb1050]

Checkstyle is fixed now as well.

[jbertram]

For what it's worth you can run checkstyle locally using:

mvn -Pdev install

See the hacking guide for more details.

[joshb1050]

@jbertram thank you—it's strange since I did run mvn checkstyle:check and mvn -Pdev install and both passed even with the checkstyle error locally. It's also unclear given that check passed for 2 of the 3 failures. I have corrected the latest one that I see on CI though.

[joshb1050]

Hi @jbertram, any feedback on this?

[gemmellr]

@jbertram thank you—it's strange since I did run mvn checkstyle:check and mvn -Pdev install and both passed even with the checkstyle error locally. It's also unclear given that check passed for 2 of the 3 failures. I have corrected the latest one that I see on CI though.

The last failure you had on 1 of the 3 'check' jobs, was actually from ErrorProne rather than Checkstyle, due to missing an @ Override. There is a different ErrorProne profile used on JDK 11+ builds and JDK16+ (i.e the JDK 17 and 21 CI runs), and that latter profile also does not check the @ Override condition, because a Java 15 class signature change meant that one (or more, I forget) of the classes in the codebase actually violates that on Java 15+ but not on earlier versions (aside: I deliberately left 15 hanging when adapting the other profile for 16/17+, since it was out of support anyway).

This came up because in the past, the JDK > 11 runs were all using their own newer compile signatures, because we couldnt set the maven.compiler.release flag at the time. That is actually recently in place, so theyll all now be using the JDK11 method signatures now, hopefully meaning that issue shouldnt be present any more and I can adjust it to error on missing @ Override again in the JDK16+ profile too for consistency.

[gemmellr]

ErrorProne check on JDK16+ adjusted in 6584220

[joshb1050]

Hi @gemmellr @jbertram can I please get a re-review on this?

[gemmellr]

if the validator is being called POSITIVE_POWER_OF_TWO would it make sense for the exception bundle method to be specific as well?

[gemmellr]

Spotted a few small niggles, nothing serious.

Would like Justin or someone else more familiar to also review.

[jbertram]

Generally speaking I think this looks good. However, there's a couple of things I would revisit:

1. The name intermediate-message-buffer-initial-size, while technically accurate, is probably not terribly clear to users. I think something like initial-queue-buffer-size is more clear.
2. The documentation defines this as the " initial number of elements of the intermediate message reference buffer." Again, this is technically accurate, but it doesn't give users a clear reason why they might want to configure it. The docs should make clear that this is tied directly to the number of bytes from the JVM's heap each queue will consume initially. The docs might also mention the canonical use-case for this (i.e. lots of queues which might remain empty or have a small number of messages).

[joshb1050]

@jbertram I agree, that makes sense. I have revised to initial-queue-buffer-size and changed usages, and also added additional context on when one might want to configure.

[gemmellr]

LGTM

[jbertram]

@joshb1050, thanks for the contribution!