Public access to mutable fields in config objects

[georgespalding]

Most classes in package com.aerospike.client.policy such as ClientPolicy, Policy and subclasses have public access on all their members, and these members are mutable.

Being new to Aerospike in general and to it's java client in particular, I am still terrified by this configuration model. I am sure you also are aware of the pitfalls of global mutable state and concurrency. Since changing any of these values while the client is running in a multithreaded environment is guaranteed to cause issues/undefined behaviour, I strongly recommend refactoring the policy package to use Builders that produce immutable Policy objects.

Would you be interested in a pullrequest for this? (If it should be on the 4.1 track or 5.0 is up to you)

[BrianNichols]

The current model implies that a copy of the policy instance must be created whenever the policy needs to change from the cluster default (copy on write). Each policy has a copy constructor for this purpose. If this one rule is followed, there will be no problems with concurrent policy modifications.

Your solution is desirable in that the user is forced to follow this rule, but it comes at a cost. It would require duplicating each policy class with an analogous policy builder class. Policy modifications would now:

1) Copy cluster default policy fields to policy builder fields with a copy constructor. 2) Change applicable policy builder field(s). 3) Copy policy builder fields to immutable policy with "toPolicy()" method.

This solution adds boilerplate code and requires extra instructions. My personal opinion is that the cost is greater than the benefit.

[georgespalding]

ClientPolicy seems to lack the alleged copy constructor you are talking about, but has lots of mutable fields. I firmly believe in creating api:s that can't be misused or cause side effects when used inappropriately is worth the extra boilerplate code and extra instructions. See also https://stackoverflow.com/a/554822/696508

The builders would only be used before creating the client, and all the code (that we want to be fast) will only use the built ClientPolicy and Policy subclasses.

[BrianNichols]

Copy constructors have been added for policy classes that did not have them.

http://www.aerospike.com/download/client/java/4.0.8