ISPN-7641 Do not use the Triangle when in server mode

[pruivo]

https://issues.jboss.org/browse/ISPN-7641

[pruivo]

run performance tests please
cs

[ghost]

Performance tests didn't finish successfully. @Holmistr, can you review it?

[Holmistr]

@pruivo Investigating ...

[pruivo]

@Holmistr not needed. I found a problem :)

[Holmistr]

@pruivo We're well oiled machine ;-)

[pruivo]

run performance tests please
cs

[ghost]

Performance tests run successfully. Link to the results here.

[pruivo]

updated with JIRA and inverted the condition.

[pruivo]

run performance tests please
cs

[ghost]

Performance tests run successfully. Link to the results here.

[pruivo]

run performance tests please
library dist writes

[ghost]

Performance tests run successfully. Link to the results here.

[Sanne]

Don't you think it's going to be confusing to have different "interpretations" of the same configuration files?

I think I'd rather have an explicit option, and the configuration files provided for server mode to use different - yet explicit - configuration regarding this.

[tristantarrant]

To be honest, I'd rather not add explicit configuration for this: we already have too many options. I think this falls under the "ergonomics" category, i.e. choose one strategy over another based on the environment. Placing the burden on the user to understand yet another option is even less ideal than the solution used here
Ideally I'd prefer the triangle code to have code paths for this specific case, but for now this will do until we find a better solution.

[Sanne]

Fair enough, but will we have some debugging facility to know what Infinispan is configured to do?

i.e. show the operating mode over JMX ?

I'd not want to get back into the "dark ages" in which I was not able to help anyone using Infinispan configured via WildFly's own configuration mechanism because the defaults would not be the same as I was used to...

[danberindei]

+1, I'd like to keep this in the configuration as "computed attributes" that cannot change. (Not sure if they should be in the builder at all, maybe only in the configuration.) That would be nice for the write skew check and versioning settings as well.

[pruivo]

@Sanne the interceptor list is printed in the beginning when in debug mode. If it has any triangle interceptor, it is using the triangle, otherwise it is not.

[pruivo]

Maybe it is worth to say that I don't want this to be a definitive solution. I would like to improve the triangle in order to user per-key versioning to order the updates between primary->backup (instead of per-segment ordering). This would achieve similar performance to the old algorithm and it can be used server mode again.

[danberindei]

@pruivo @tristantarrant WDYT about adding a generic PrivateConfiguration module configuration for settings that we don't want to expose in the configuration because they're experimental or only useful in a very restricted set of circumstances?

[tristantarrant]

Yes, I like it.

[Sanne]

+1 it's probably worth having a generic "conventional" way of expressing any such configuration property which we don't really want to expose.. as this won't be a unique case I guess. It would actually be nice to migrate more "ergonomic options" to maybe a read-only section of the configuration?

Just please don't have people re-run their app with debug level just to figure out how it's configured, but I don't mean this "wish" to block this PR.. just a suggestion to improve on for the future headaches prevention.

[pruivo]

what do you suggest @Sanne?

[Sanne]

1. To ignore me for now and move on ;)

2. and then follow @danberindei 's suggestion, hopefully in a timely way so that this won't cause too much confusion.

3. document the approach for "private configurations" in a wiki and recommend everyone to take a look to other configuration attributes which could benefit from a similar approach. E.g. I'm thinking in Query we could also simplify a lot of stuff, I never wished to take away some "advanced options" as in some corner cases the options are needed, but a simplification would be welcome.

[pruivo]

@Sanne @danberindei @tristantarrant I've added the PrivateGlobalConfiguration. let me know what you think.

[tristantarrant]

probably it makes sense to exclude this package from javadocs using the @Private doclet

[pruivo]

updated!

[pruivo]

run performance tests please
cs dist writes

[tristantarrant]

I see there are some failures related to joiners / leavers, but I doubt they are related

[tristantarrant]

Ok, verified locally and everything works well.

[tristantarrant]

Merged. Thanks @pruivo

[ghost]

Performance tests didn't finish successfully. @Holmistr, can you review it?

[Holmistr]

@pruivo The perf test fail is not related to this PR, just FYI. As this is integrated now, I won't retrigger the tests.