Hello everyone,
I would like to share and discuss the key point of the solution design with you before I finalise a pull request with tedious changes remaining so that we are all on the same page with the changes to the valuable Config class and its accessors. Here is the issue I'm working on: "Allow UPDATE on settings virtual table to change running configurations". https://issues.apache.org/jira/browse/CASSANDRA-15254 Below is the restricted solution design at a very high level, all the details have been discussed in the related JIRA issue. = What we have now = - We use JMX MBeans to mutate this runtime configuration during the node run or to view the configuration values. Some of the JMX MBean methods use camel case to match configuration field names; - We use the SettingsTable only to view configuration values at runtime, but we are not able to mutate the configuration through it; - We load the configuration from cassandra.yaml into the Config class instance during the node bootstrap (is accessed with DatabaseDescriptor, GuardrailsOptions); - The Config class itself has nested configurations such as ReplicaFilteringProtectionOptions (it is important to keep this always in mind); = What we want to achieve = We want to use the SettingsTable virtual table to change the runtime configuration, as we do it now with JMX MBeans, and: - If the affected component is updated (or the component's logic is executed) before or after the property change, we want to keep this behaviour for the virtual table for the same configuration property; - We want to ensure consistency of such changes between the virtual table API and the JMX API used; = The main question = To enable configuration management with the virtual table, we need to know the answer to the following key question: - How can we be sure to determine at runtime which of the properties we can change and which we can't? = Options for an answer to the question above = 1. Rely on the volatile keyword in front of fields in the Config class; I would say this is the most confusing option for me because it doesn't give us all the guarantees we need, and also: - We have no explicit control over what exactly we expose to a user. When we modify the JMX API, we're implementing a new method for the MBean, which in turn makes this action an explicit exposure; - The volatile keyword is not the only way to achieve thread safety, and looks strange for the public API design point; - A good example is the setEnableDropCompactStorage method, which changes the volatile field, but is only visible for testing purposes; 2. Annotation-based exposition. I have created Exposure(Exposure.Policy.READ_ONLY), Exposure(Exposure.Policy.READ_WRITE) annotations to mark all the configuration fields we are going to expose to the public API (JMX, as well as the SettingsTable) in the Config class. All the configuration fields (in the Config class and any nested classes) that we want to expose (and already are used by JMX) need to tag with an annotation of the appropriate type. The most confusing thing here, apart from the number of tedious changes: we are using reflection to mutate configuration field values at runtime, which makes some of the fields look "unused" in the IDE. This can be not very pleasant for developers looking at the Config class for the first time. You can find the PR related to this type of change here (only a few configuration fields have been annotated for the visibility of all changes): https://github.com/apache/cassandra/pull/2133/files 3. Enforce setter/getter method name rules by converting these methods in camel case to the field name with underscores. To rely on setter methods, we need to enforce the naming rules of the setters. I have collected information about which field names match their camel case getter/setter methods: total: 345 setters: 109, missed 236 volatile setters: 90, missed 255 jmx setters: 35, missed 310 getters: 139, missed 206 volatile getters: 107, missed 238 jmx getters: 63, missed 282 The most confusing part of this type of change is the number of changes in additional classes according to the calculation above and some difficulties with enforcing this rule for nested configuration classes. Find out what this change is about here: https://github.com/apache/cassandra/pull/2172/files = Summary = In summary, from my point of view, the annotation approach will be the most robust solution for us, so I'd like to continue with it. It also provides an easy way to extend the SettingTable with additional columns such as runtime type (READ_ONLY, READ_WRITE) and a description column. This ends up looking much more user-friendly. Another advantage of the annotation approach is that we can rely on this annotation to generate dedicated dynamic JMX beans that only respond to node configuration management to avoid any inconsistencies like those mentioned here [2] (I have described a similar approach here [1], but for metrics). But all this is beyond the scope of the current changes. Looking forward to your thoughts. [1] https://lists.apache.org/thread/26j9hhy39okw0wy79mtylb753w6xjclg [2] https://issues.apache.org/jira/browse/CASSANDRA-17734
