Move compaction service properties to accumulo props file

[ddanielr]

Is your feature request related to a problem? Please describe. There are a number of compaction service properties that are required for the system to function. They have been placed in Property.java with default values that work for a given system.

This design choice causes unnecessary complications when attempting to modify compaction service code; especially while supporting deprecation in one accumulo version and addition of a new property since both sets of properties cannot be defined at the same time.

This is further compounded by the fact that they are parsed in sections by the DefaultCompactionPlanner. This parsing means no direct references to these properties exist in the code base. This lack of references could cause someone to assume these properties could be removed without issue.

Describe the solution you'd like Move these properties to the accumulo.properties file and assign them their current default values.

This ensures that the properties are exposed to the user in a more direct method while providing us flexibility in our code base to change compaction service code.

[ctubbsii]

The way this is written is very confusing to me. I read it as moving properties from Property.java to accumulo.properties. However, Property.java is just an enum that helps us store property documentation, default values, and more that help us read the properties from an accumulo.properties file (or from properties stored elsewhere on the system). So, they aren't separate things... they go together. The accumulo.properties file you linked to is merely a reference for a relatively minimal implementation.

I would not want the minimal implementation bloated with extraneous configs. But, I'm not sure if that's what you're suggesting, or if you're suggesting something else.

There are some elements of this that I'd be in favor of:

• Changing the default values to an empty string
• Making sure the properties are referenced by the compaction code instead of only placed for documentation and not actually retrieved via the enum references

There are also some related things that could help here:

• Use "general.custom." or "table.custom." prefixes for pluggable implementations that provide complex, non-default features (see VolumeChooser subclass configs, or other SPIs that behave similarly)
• Relax our constraint that non-core properties be prefixed with "general.custom." prefix, or "table.custom." prefix to be stored in our config files (but this means we need to be more rigid about the property namespace we do expect to be reserved for built-in properties)
• Combine complex config into a single (possibly optional) property for customization (AWS uses this kind of pattern extensively in its APIs)
• Wrap complex features into a pluggable component; users can choose to hard-code in the complexity, or make their implementation of that component customizable through config; we choose a simple implementation that requires no config (see also VolumeChooser implementations for a similar pattern, or our general context class loader factory, for an even simpler implementation of this).

[ddanielr]

I tried just emptying the default compaction values for these properties and ran into a LOT of validation errors that were only solved by poking holes in the current validation and test code. We would run into those problems even if these properties fell under the general/custom prefixes.

I'm happy to create a separate PR with the changes needed for empty property definitions in Property.java to compare the differences. This may lead to refactoring Properties to support some sort of example flag/option that doesn't result in reducing our ability to validate properties.

I agree that having the empty values in Property.java is a better goal as it allows us to use the PropertyType.json and have stronger property validation. Also automatic property documentation is generated from the entries in Property.java.

[ddanielr]

So I took some more time with this and figured out a way to add example properties to Property.java.

I didn't want to add defaultValues for properties that we actually didn't want set. There has been code before that checks to see if the user set the property and only does things if that is true vs the property being in the default configuration.

That pattern seems to add additional complexity to our code while also confusing users who might set the property just because we have it listed as a default and they want to hedge against the property changing in the default configuration.

So I added two things. The first is an @Example annotation which marks the property has an example prop. This means the property is excluded from the DefaultConfiguration generation and also adds different property documentation details. The second is another value field called exampleValue. This was done to ensure that if an annotation is mistakenly removed, the property still has no way of impacting a system.

The annotation must be removed, and the default value updated in order for the property to be included in the DefaultConfiguration.