[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[mtaylor]

Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
> @mtaylor I agree, having percentages is better, and will work for our use 
case.

Great.  It took me 5 minutes to write that reply and 15 minutes trying to 
get the formatting correct :p.  

@franz1981 this is inline to what we already discussed yesterday on IRC.  
You OK to update using this method?  Cheers


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[lulf]

Github user lulf commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@mtaylor I agree, having percentages is better, and will work for our use 
case.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[mtaylor]

Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
> @michaelandrepearce @mtaylor Ok, just to make sure I understand this, 
what we have today:
> 
> ```
> -1
> ```
> would be equivalent to:
> 
> ```
> #
> -1
> ```
> Which would give the same 1/2 JVM heap as the max size.
Let's separate out the two use cases.

1. Let the broker choose suitable numbers for the total memory size.  Which 
is what the global-max-size=-1 does here.  Currently it's set at 0.5 x heap 
size.  

2. Allow flow of management messages when all your memory utilisation for 
your client address space is exhausted.

1. I would suggest we leave the setting as it is.  But I don't like the -1 
setting here.  As we use this to mean OFF in other places of the config.  
Something like 100% would be better.

2. I think there are two ways of looking at the problem, reserving 
capacity, restricting usage.  They both solve the original use case but come 
from different ends.  What I originally meant was restrict usage, i.e. prevent 
other address spaces from taking up all the resources.  This is inline with the 
address size setting.  To do this, you can set the size in terms of memory or 
as a percentage of the overall memory size (inline with the setting from 1.).  
For your use case you could do:

foo.#
90%

bar.#
-1 // i.e. off.

This essentially means, restrict the foo.# address space to 90% of the 
overall capacity, ensuring that 10% is always available for the bar.# 
addresses.  At some point though, it is possible to hit that global-max-size 
limit, which is really there to protect the broker from serious problems, like 
OOM.




bar.#
-1


> 
> And to avoid restrictions on activemq.management we would set:
> 
> ```
> #
> -1
> 
> activemq.management
> 0 
> ```
> What would be a bit odd is the following:
> 
> ```
> foo.#
> -1
> 
> 
> bar.#
> -1
> ```
> Perhaps `-1` should only be allowed once, alternatively only on "#"?




---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@lulf @michaelandrepearce I was looking to implement it as:
```xml
30
```
Per address-settings to allow all the addresses matching it to share a 30% 
of the configured global-max-size.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
I would suggest for address settings -1 isnt used (as for other settings it 
typically means disable also for your above mentioned it could not be very 
flexible) but some expression (take inspiration from spring spel) is 
implemented could be used to deduce the calculated. So if you wanted to have 
different groups you could split up the allocation by a percentage or some 
other calc. 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[lulf]

Github user lulf commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@michaelandrepearce @mtaylor Ok, just to make sure I understand this, what 
we have today:

```
-1
```
would be equivalent to:
```
#
-1
```

Which would give the same 1/2 JVM heap as the max size.

And to avoid restrictions on activemq.management we would set:
```
#
-1

activemq.management
0 
```

What would be a bit odd is the following:
```
foo.#
-1


bar.#
-1
```

Perhaps `-1` should only be allowed once, alternatively only on "#"?


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@clebertsuconic the change you propose would mean still management 
addresses would NOT be under the global max catch all, as it is today. Which is 
my initial concern and my negative vote for the change in behaviour, as we 
explitily rely on that heavily today with its current  behaviour.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@lulf if i understand @mtaylor proposal it would be that you could set 
address setting match to # meaning anything and set aggegate-size to the max 
size you want for all normal addresses. And then youd have a specific address 
setting that matches the management addresses that would have a seperate 
aggregate size. Thus meaning the space for management is seperated so its space 
wouldnt be consumed by the others


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@clebert I will try to connect later in order to understand what you mean 
mate (ATM I'm on the phone)


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@lulf / @franz1981  ok, 

Why not make a simpler change.. and return -1 on PaginManager if the 
management address. That's a lot less code to get around the sizing.


@franz1981 Also, please rename the commit and JIRA to *exceed* size. 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[jbertram]

Github user jbertram commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
FWIW, management can be done through JMX or REST (via Jolokia) if 
management messages are blocked for any reason.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[lulf]

Github user lulf commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
The use case is covered in 
https://issues.apache.org/jira/browse/ARTEMIS-1710

In a cloud-setting, we need to:

a) Have the broker derive (global) limits based on things like heap memory 
in order to avoid falling over. This may be configured statically in the 
broker.xml
b) Allow management messages to get through even if other addresses are 
blocked by global-max-size (using BLOCK/FAIL policy, we do not wish to use 
PAGE) so that our system can keep operating even if queues are full (i.e. we 
can fetch queue stats and report back to user always)

Also, keep in mind that:

a) We don't know what addresses will exist beforehand and they are created 
via management address.
b) We want to avoid setting limits per address via management, as that gets 
super slow (I know this was confirmed by someone a while back, i don't know the 
jira number). Perhaps the address-space limits proposed by @mtaylor would work 
around that allowing us to set it statically for all addresses and make 
management an exception in our configuration. I'm not sure how that would work 
with not knowing the limits beforehand, or if it would be possible to 'disable' 
the limit for management addresses by setting it to some special value.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@franz1981 Did you mean exceed global-max-size?

when I read this originally I had the impression you were setting 
global-max-size using management messages.

I'm still trying to get my head around basic use cases.. such as.. why not 
simply set the management address size as -1? 

I'm confused on basic stuff here.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[mtaylor]

Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
> This would work and i like the flexibility. I assume here in this option 
proposed. Global would remain as is today.

Yes. 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@michaelandrepearce @lulf @mtaylor I will continue on the issue comments 
:+1: 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@mtaylor @michaelandrepearce I can't ignore all these useful 
comments/feedbacks guys so I will work more on this one to see if it can be 
done in a better way; thanks for the reviews :+1: 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
This would work and i like the flexibility. I assume here in this option 
proposed. Global would remain as is today.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[mtaylor]

Github user mtaylor commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
I don't really like the idea of the double threshold.  I think the 
global-max-size limit should be the catch all case.  

What would work better here is an  accumulative-max-size setting that 
applies to an address space that limits the total amount of memory used by all 
addresses that match that address setting.  Right now the limits are on a per 
address basis, but many addresses can match the same address space.  

Doing it this way allows a lot more flexibility in the broker, users can 
limit certain address spaces for other reasons that to enable management 
messages to flow, like for example, reserving capacity for critical 
applications and blocking others.  e.g. telemetry.# vs command.# 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@clebertsuconic I have added some additional unit tests on `PagingStoreImpl`


---

Re: [GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[CodeTechGuy]

That is a sagaciously composed article and this is actually what I was
searching for. Great motivational counsel, and  more info
   just
as exactly what I require. I altogether making the most of your top to
bottom audit and have found the answer i was looking for.



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@clebertsuconic I'm fixing a couple of things on this one too


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[lulf]

Github user lulf commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
As a user, I think the first alternative is the clearest, it explicitly 
states the intention and does not change any existing behavior (i.e. auto tune 
-1 which we already rely on). In our case, only trusted users can send messages 
to management, so we are willing to take the 'risk'.


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@michaelandrepearce I've started working on the alternative one based on a 
second level global limit, but TBH I've already found quite complex to add such 
condition without breaking any of the current behaviours.
I'm evaluating if it is really the case to add a more precise feature whose 
real use case would be the same of this one while involving a similar level of 
dangerousness.
To obtain something similar to:
 ```xml
Any value or left it -1 to be auto-tuned

true
```
the alternative solution will need to configure:
 ```xml
The original value used for 
global-max-size
A value that is assumed to be unlimited
```
 Only this configuration can give the 100% guarantee to the user that any 
management message won't be paged/fail/block regardless the current size of the 
non-management addresses.
This type of configuration is dangerous for the same reasons this same PR 
is.
In addition it will need to add:

- a new value that identify an unlimited value, because -1 for 
`global-max-size` will force it to be auto-tuned
- a way to auto-tune the `global-non-management-max-size` too (as we're 
doing it for `global-max-size`)
- an impact on any code paths of the check that we perform now


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@clebertsuconic Let me run it while I'm working on an alternative version 
too: we can talk later about it mate?


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[clebertsuconic]

Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@franz1981 did you run a full testsuite on this?


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[franz1981]

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
> Surely this would be better handled with simply setting better values per 
address. 

I could be wrong (and @mtaylor or @clebertsuconic can help me to understand 
the conditions I have found in the code), but it seems to me that the 
global-max-size can never be surpassed, regardless any configuration you'll set 
for a maching address.
@lulf in the referenced issue has explained it very clearly: if you desire 
to have a global-max-size to be shared between all the addresses, but you need 
management messages to not being influenced by it, it doesn't seem possible 
with the current broker configuration.
By default this option is disabled ie management addresses behave like we 
are always used to see.

> Or at least implementing a two tier max, keeping the existing 
global-max-size as behaviour today (so users dont get a change in expectation), 
but implementing a secondary value thats global-non-management-max-size that 
could be a catch all at a level below.

That's a good point: I like the idea of a `global-non-management-max-size` 
(while leaving `global-max-size` unbounded): what makes me not 100% sure of it 
is that to satisfy that same requirement it will introduce  a whole new 
feature, probably bigger. Let me think about it and hear the others opinions on 
it :+1: 


---

[GitHub] activemq-artemis issue #2401: ARTEMIS-1710 Allow management msgs to pass the...

[michaelandrepearce]

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2401
  
@franz1981 this seems a bit dangerous, as a user/operator i set global max 
as a safety net even for management messages. Surely this would be better 
handled with simply setting better values per addresses. 

Or some other way to handle such needs of sorting the broker out like using 
JMX and Management Console.


---