Remove default limit for the number of visible instances on MC

[emre-aydin]

hazelcast.mc.max.visible.instance.count group property is used for
limiting the number of visible instances on Management Center. Its
default value is updated to Integer.MAX_VALUE, effectively removing
the limit if it's not configured explicitly. The current value of 100
confuses users when it's exceeded, as the users don't configure it. Should
it be needed to limit the number of visible instances, it can still be
configured to a lower number.

Also fixes #12095

[emre-aydin]

Documentation update: hazelcast/hazelcast-reference-manual#474

[jbee]

I am surprised by the use of List. Is there an important order to the members I am unaware of?
The name longInstanceNames seams confusing. I'm guessing the names are of any length but it might become a long list or a large set?
Also the implementation does not really prevent the list from becoming larger than the maximum set.

I also wonder why we would have the limit at all if its default is to not have it. Why was it introduced in the first place? Don't those reasons apply any longer? Because it seams some soft of protection.
Wouldn't it be the best if there is a limit but if reached the user sees a message directly at the end of the list that it has been shortened for reason X and that this can be configured using this setting?

[jbee]

See comment above ;)

[emre-aydin]

Is there an important order to the members I am unaware of?

Nope. It's used just because we don't know the number of elements in advance (so pre-allocation is not possible) and the only operations on them is adding them together and sending them to Management Center as JSON by iterating the list.

The name longInstanceNames seams confusing.

Agreed. I think it has to do with the names having their prefixes in that list. I made no changes to it. I'd like to keep the PR only about the exact change I'm making, which is changing the default value and fixing a bug related to an out of memory error (see the linked issue in the PR description for that).

I also wonder why we would have the limit at all if its default is to not have it. Why was it introduced in the first place?

I believe it was introduced to prevent a very large set of data flowing from the member to Management Center. Can't remove because of backwards compatibility, and also there might be people who set it to some value other than the default value to limit the number of visible instances on Management Center (although admittedly I've never seen anyone do this yet).

Also the implementation does not really prevent the list from becoming larger than the maximum set.

It does, why do you think it doesn't? Is there something I'm missing?

Wouldn't it be the best if there is a limit but if reached the user sees a message directly at the end of the list that it has been shortened for reason X and that this can be configured using this setting?

The PRD for this work requires us to update the default value. This might be done as a separate enhancement. It also has an issue on Management Center repository: https://github.com/hazelcast/management-center/issues/536

[pveentjer]

What is the reason behind the original limit on returning a maximum number of members? AFAIK it isn't a big issue since I have not seen many clusters with more than 100 members.

Apart from this question, the PR looks good to me.

[emre-aydin]

@pveentjer the wording is a bit confusing in this case. instance is used as a synonym for "distributed data structure" (i.e. not members) so it limits the number of distributed data structures that we send statistics for (to Management Center).

[jbee]

Nope. It's used just because we don't know the number of elements in advance (so pre-allocation is not possible)

In that case a ArrayList is still better for the sizes we expect but a HashSet (with no initial capacity set) is fine as well if we think a Set expresses the intent better. If 100 members is a big installation it really does not matter performance-wise and the collection that expresses the intent best is probably the best choice.
In general: LinkedList are mostly useful in for lists that are modified randomly (both in time and operation). For a uniformly growing list usually ArrayList is preferable as it is faster and avoids memory fragmentation.

It does, why do you think it doesn't? Is there something I'm missing?

I missed that the handleX methods also increased and checked the count internally. It was unexpected behaviour looking at the call site.

I believe it was introduced to prevent a very large set of data flowing from the member to Management Center.

If I read @pveentjer right such large sets practically don't exist. So the only reason to have the setting is backwards compatibility as far as I understand it.

[pveentjer]

The naming is certainly confusing. The list doens't contain only member data, but it also contains all data for all distributed datastructures. So if you have 10 member cluster and 100 maps, then the returning object will contains at least 110 entries.